Chiral GNRs and [n]-triangulene #644

tfrederiksen · 2023-11-04T12:24:28Z

Following #636 I would like to propose another two characteristic honeycomb geometries, namely chiral-GNRs [characterized by the chirality index (n, m)] as well as [n]-triangulene.

Added tests for new/changed functions?
Ran isort . and black . at top-level
Documentation for functionality in docs/
Changes documented in CHANGELOG.md

Examples

import sisl
import sisl.viz
sisl.geom.cgnr(3, 2, 12).plot(axes='xy', atoms_scale=0.6, nsc=[4, 1, 1])

g = sisl.geom.nanoribbon(8, bond=1.6, atoms=("B", "N"), kind="chiral", index=(3, 1))
g.plot(axes='xy', atoms_scale=0.6, nsc=[4, 1, 1])

sisl.geom.triangulene(3).plot(axes='xy', nsc=[2, 2, 1])

sisl.geom.triangulene(6, bond=1.6, atoms=("B", "N")).plot(axes='xy', atoms_scale=0.6)

pfebrer · 2023-11-04T13:08:33Z

They look cool :)

Actually the way I built the flake was by first creating a triangulene:

sisl/src/sisl/geom/flat.py

Lines 108 to 145 in 24c6406

    
           # Function that generates one of the six triangular portions of the 
        
           # hexagonal flake. The rest of the portions are obtained by rotating 
        
           # this one by 60, 120, 180, 240 and 300 degrees. 
        
           def _minimal_op(shells): 
        
               # The function is based on the horizontal lines of the hexagon, 
        
               # which are made of a pair of atoms. 
        
               # For each shell, we first need to complete the incomplete horizontal 
        
               # lines of the previous shell, and then branch them up and down to create 
        
               # the next horizontal lines. 
        
               # Displacement from the end of one horizontal pair to the beggining of the next 
        
               branch_displ_x = bond * 0.5  # cos(60) = 0.5 
        
               branch_displ_y = bond * 3**0.5 / 2  # sin(60) = sqrt(3)/2 
        
               # Iterate over shells. We also keep track of the atom types, in case 
        
               # we have two different atoms in the honeycomb lattice. 
        
               op = np.array([[bond, 0, 0]]) 
        
               types = np.array([0]) 
        
               for shell in range(shells): 
        
                   n_new_branches = 2 + shell 
        
                   prev_edge = branch_displ_y * (shell) 
        
                   sat = np.zeros((shell + 1, 3)) 
        
                   sat[:, 0] = op[-1, 0] + bond 
        
                   sat[:, 1] = np.linspace(-prev_edge, prev_edge, shell + 1) 
        
                   edge = branch_displ_y * (shell + 1) 
        
                   branches = np.zeros((n_new_branches, 3)) 
        
                   branches[:, 0] = sat[0, 0] + branch_displ_x 
        
                   branches[:, 1] = np.linspace(-edge, edge, n_new_branches) 
        
                   op = np.concatenate([op, sat, branches]) 
        
                   types = np.concatenate( 
        
                       [types, np.full(len(sat), 1), np.full(len(branches), 0)] 
        
                   ) 
        
               return op, types

And then duplicate it 6 times rotating 60º. So probably the two geometries can share some code. In fact the flake could be defined as a composite_geometry where we take the triangulene and perform the rotation actions on it. It could be a nice case to play with composite_geometry.

Regarding the API, could it make sense to have a general honeycomb_flake that accepts the shape (e.g. "triangle", "hexagon"). I don't know if that would work, just an idea 😅 Basically I'm afraid that naming the hexagonal one honeycomb_flake took a name that could have been used more generally.

pfebrer · 2023-11-04T13:12:40Z

Well, it's not exactly a triangulene the thing I build, because it has one extra atom at each vertex of the triangle 😅 But still probably they can share code.

tfrederiksen · 2023-11-04T13:14:21Z

@pfebrer , sounds like a good idea if some code can be shared.

However, from the user's perspective I think it would be preferred to have sisl.geom.triangulene(3) available, rather than sisl.geom.graphene_flake(shape="triangle"), even if the actual geometry is the same.

src/sisl/geom/tests/test_geom.py

+    assert g.na == 22
+    g = triangulene(3, atoms=["B", "N"])
+    assert g.atoms.nspecie == 2
+    g = triangulene(3, bond=1.6)


src/sisl/geom/tests/test_geom.py

pfebrer · 2023-11-04T13:18:16Z

However, from the user's perspective I think it would be preferred to have sisl.geom.triangulene(3) available, rather than sisl.geom.graphene_flake(shape="triangle"), even if the actual geometry is the same.

Yes, I agree 👍

But triangulene could simply be an alias. But still maybe not worth it as different shapes probably have different arguments.

I guess the main question is if you think it makes sense that graphene_flake is hexagonal or it should be renamed to something else. I don't know if the hexagonal ones have a particular name.

pfebrer · 2023-11-04T13:21:56Z

By the way, being triangulene a 0D structure probably it would make sense that the cell was orthorombic (?) Or maybe I should change the cell in the hexagonal ones.

zerothi

Some comments, looking very good! Many comments are sort of related, so I guess the first one to decide is the interface.

src/sisl/geom/nanoribbon.py

src/sisl/geom/flat.py

tfrederiksen · 2023-11-04T20:11:04Z

By the way, being triangulene a 0D structure probably it would make sense that the cell was orthorombic (?) Or maybe I should change the cell in the hexagonal ones.

Does it matter?

tfrederiksen · 2023-11-04T22:28:31Z

Would it be more natural to use nonorthogonal cell vectors in the plane for the chiral GNRs?

g = sisl.geom.cgnr(3, 1, 8).move([3.6, 0, 0])
g.cell[1] = 26 * (g.xyz[1] - g.xyz[0])
g.plot(axes='xy', atoms_scale=0.6, nsc=[4, 1, 1])

zerothi · 2023-11-05T09:32:03Z

Would it be more natural to use nonorthogonal cell vectors in the plane for the chiral GNRs?
g = sisl.geom.cgnr(3, 1, 8).move([3.6, 0, 0])
g.cell[1] = 26 * (g.xyz[1] - g.xyz[0])
g.plot(axes='xy', atoms_scale=0.6, nsc=[4, 1, 1])

I guess so in terms of appending structures etc.

zerothi · 2023-11-06T10:16:43Z

Would it be more natural to use nonorthogonal cell vectors in the plane for the chiral GNRs?
g = sisl.geom.cgnr(3, 1, 8).move([3.6, 0, 0])
g.cell[1] = 26 * (g.xyz[1] - g.xyz[0])
g.plot(axes='xy', atoms_scale=0.6, nsc=[4, 1, 1])
I guess so in terms of appending structures etc.

I guess an argument orthogonal could handle this, no? Although there would be the same number of atoms, it might be useful?

zerothi · 2023-11-06T10:21:41Z

@tfrederiksen sorry, I messed up things! Thought I was on main, could you revert my latest commit here...

zerothi · 2023-11-06T10:23:24Z

@tfrederiksen sorry, I messed up things! Thought I was on main, could you revert my latest commit here...

I think I fixed it, let me know if I caused problems!! :)

codecov · 2023-11-06T10:29:43Z

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (5339e68) 87.65% compared to head (e3482c5) 87.67%.
Report is 1 commits behind head on main.

❗ Current head e3482c5 differs from pull request most recent head b741ac2. Consider uploading reports for the commit b741ac2 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #644      +/-   ##
==========================================
+ Coverage   87.65%   87.67%   +0.01%     
==========================================
  Files         364      364              
  Lines       48520    48566      +46     
==========================================
+ Hits        42531    42578      +47     
+ Misses       5989     5988       -1

Files	Coverage Δ
src/sisl/geom/flat.py	`100.00% <100.00%> (ø)`
src/sisl/geom/tests/test_geom.py	`100.00% <100.00%> (ø)`
src/sisl/geom/nanoribbon.py	`91.35% <96.87%> (+0.62%)`	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pfebrer · 2023-11-06T16:30:13Z

Does it matter?

Hmm I don't know, some tools don't work well with non-orthogonal cells. Some of the SIESTA utils for example. In general it is much easier to work with orthogonal cells so I think for a 0D structure where the cell doesn't matter it would be better to have an orthogonal cell. Just to make life easier for users :)

tfrederiksen · 2023-11-06T18:19:59Z

Does it matter?

Hmm I don't know, some tools don't work well with non-orthogonal cells. Some of the SIESTA utils for example. In general it is much easier to work with orthogonal cells so I think for a 0D structure where the cell doesn't matter it would be better to have an orthogonal cell. Just to make life easier for users :)

OK, I can change to an orthogonal for triangulene.

zerothi · 2023-11-06T18:20:47Z

Does it matter?

Hmm I don't know, some tools don't work well with non-orthogonal cells. Some of the SIESTA utils for example. In general it is much easier to work with orthogonal cells so I think for a 0D structure where the cell doesn't matter it would be better to have an orthogonal cell. Just to make life easier for users :)

I would still advocate the possibilities of retaining the non-orthogonal vectors when appending stuff, so could we retain both possibilities? :-)

tfrederiksen · 2023-11-06T18:25:49Z

Does it matter?

Hmm I don't know, some tools don't work well with non-orthogonal cells. Some of the SIESTA utils for example. In general it is much easier to work with orthogonal cells so I think for a 0D structure where the cell doesn't matter it would be better to have an orthogonal cell. Just to make life easier for users :)

I would still advocate the possibilities of retaining the non-orthogonal vectors when appending stuff, so could we retain both possibilities? :-)

I agree to adapt a nonorthogonal cell for the chiral ribbons. I think the comment by @pfebrer only referred to triangulene.

pfebrer · 2023-11-06T18:47:24Z

Yes, it's only for the triangulene

tfrederiksen · 2023-11-06T19:21:05Z

Regarding the listing of geometries in the documentation: What do you think about moving "bulk" and "surfaces" into new subsections of a "3D materials" category?

zerothi · 2023-11-06T19:24:02Z

Regarding the listing of geometries in the documentation: What do you think about moving "bulk" and "surfaces" into new subsections of a "3D materials" category?

A separate pr for that please... Let's get this in.

tfrederiksen · 2023-11-06T21:41:52Z

I've made some further changes according to the discussion above:

Ribbons

the positioning inside the cell is in all cases now set by geom = geom.move(geom.center(what="cell") - geom.center())
the in-plane separation is controlled by the vacuum parameter
the chirality index is now always a tuple of two integers

g = sisl.geom.agnr(7, vacuum=0)
g.plot(axes='xy', atoms_scale=0.6, nsc=[6, 2, 1])

g = sisl.geom.zgnr(5, vacuum=0)
g.plot(axes='xy', atoms_scale=0.6, nsc=[6, 2, 1])

g = sisl.geom.cgnr(8, (3, 1), vacuum=10)
g.plot(axes='xy', atoms_scale=0.6, nsc=[3, 2, 1])

g = sisl.geom.nanoribbon(4, bond=1.6, atoms=("B", "N"), kind="chiral", chirality=(3, 1), vacuum=0)
g.plot(axes='xy', atoms_scale=0.6, nsc=[4, 2, 1])

Note that for chiral ribbons with even width, setting vacuum=0 results in the recovery of the honeycomb lattice.

Triangulene

Orthogonal cell

sisl.geom.triangulene(3, vacuum=5).plot(axes='xy', nsc=[2, 2, 1])

tfrederiksen · 2023-11-06T22:00:33Z

@pfebrer my changes here generate a conflict with the tests of your heteroribbons, more specifically

    def test_heteroribbon():
        """Runs the heteroribbon builder for all possible combinations of
        widths and asserts that they are always properly aligned.
        """
        # Build combinations
        combinations = itertools.product([7, 8, 9, 10, 11], [7, 8, 9, 10, 11])
        L = itertools.repeat(2)
    
        for Ws in combinations:
            geom = heteroribbon(
                zip(Ws, L), bond=1.42, atoms=Atom(6, 1.43), align="auto", shift_quantum=True
            )
    
            # Assert no dangling bonds.
>           assert len(geom.asc2uc({"neighbours": 1})) == 0
E           AssertionError: assert 3 == 0
E            +  where 3 = len(array([ 0, 19, 22]))
E            +    where array([ 0, 19, 22]) = <bound method Geometry.sc2uc of <sisl.Geometry na=30, no=30, nsc=[3 1 1]>>({'neighbours': 1})
E            +      where <bound method Geometry.sc2uc of <sisl.Geometry na=30, no=30, nsc=[3 1 1]>> = <sisl.Geometry na=30, no=30, nsc=[3 1 1]>.asc2uc

Some combinations in the for loop are OK, but the first that fails ("dangling bond cases") is this:

It seems that my relocation of ribbons (to be completely inside the cell) is now breaking your routines. Do you see where?

zerothi

I can't immediately see where things goes wrong, the only thing you are changing are the vacuum distances... Hmm..

src/sisl/geom/flat.py

src/sisl/geom/nanoribbon.py

tfrederiksen · 2023-11-07T10:33:28Z

I can't immediately see where things goes wrong, the only thing you are changing are the vacuum distances... Hmm..

I am also shifting the whole geometry in the cell (no atoms at the borders), by introducing this call:

geom = geom.move(geom.center(what="cell") - geom.center())

zerothi · 2023-11-07T10:33:47Z

I can't immediately see where things goes wrong, the only thing you are changing are the vacuum distances... Hmm..

I am also shifting the whole geometry in the cell (no atoms at the borders).

that might be the reason...

pfebrer · 2023-11-07T21:09:46Z

Ok, so apparently the vertical shift is fine, the only thing that broke is the longitudinal distance. It makes sense that it has changed if the original ribbons have been shifted, probably I just hardcoded their position longitudinally. Let me check.

pfebrer · 2023-11-07T21:21:29Z

If it's a constant shift for all cases, I guess it would be enough to apply the opposite shift here:

sisl/src/sisl/geom/nanoribbon.py

Lines 334 to 336 in f06570c

    
           new_section = nanoribbon( 
        
               bond=self.bond, atoms=self.atoms, width=self.W, kind=self.kind 
        
           )

But after giving it a thought, if it broke the generation of heteroribbons it is very likely that it will break other scripts. Is it really necessary to center along the longitudinal axis, or is it just an aesthetical choice? Because this is a backwards uncompatibility that is really hard to track an one can not really warn the users.

…rds compatibility)

pfebrer · 2023-11-08T14:13:06Z

How does vacuum work with the heteroribbons? Is it vacuum with respect to the most external edges of the ribbon?

Could you add the parameter to the docstring of _heteroribbon_section? :)

zerothi · 2023-11-08T14:18:07Z

These vacuum arguments are nice, on a next iteration it would be ideal if users can control the vacuum for all non-PBC directions explicitly. Currently, all sisl.geom methods are not coherent in terms of what vacuum means.
It would be ideal if vacuum could be a float | Sequence[float] so it automatically broadcasts to the non-PBC dimensions. Perhaps not for this PR, but for another one :)

zerothi · 2023-11-08T14:25:39Z

I'll merge once @pfebrer and @tfrederiksen have reached agreement :)

tfrederiksen · 2023-11-08T14:39:20Z

@pfebrer , thanks, I've now added the docstring.

vacuum for heteroribbons is with respect to the widest segment, e.g.,

g = sisl.geom.graphene_heteroribbon([(5,2), (18, 2, -2), (7, 2), (11, 2)], kind='armchair', vacuum=0)
g.set_nsc([3, 3, 1])
g.plot(axes="xy", atoms_scale=0.6, nsc=[3, 2, 1])

and for vacuum=10:

tfrederiksen · 2023-11-08T14:40:22Z

These vacuum arguments are nice, on a next iteration it would be ideal if users can control the vacuum for all non-PBC directions explicitly. Currently, all sisl.geom methods are not coherent in terms of what vacuum means. It would be ideal if vacuum could be a float | Sequence[float] so it automatically broadcasts to the non-PBC dimensions. Perhaps not for this PR, but for another one :)

It sounds like a good idea to try to unify this (in another PR).

pfebrer · 2023-11-08T14:41:03Z

It works also if the widest segment has one of the edges "inside" the ribbon? E.g. if you shift up the 18 ribbon.

pfebrer · 2023-11-08T14:44:52Z

By the way, nice unexpected new feature of heteroribbon that you showed here:

You can create nanoporous graphene with arbitrary pore shapes, we should add it to the CHANGELOG 😆

tfrederiksen · 2023-11-08T14:45:43Z

It works also if the widest segment has one of the edges "inside" the ribbon? E.g. if you shift up the 18 ribbon.

Like this?

g = sisl.geom.graphene_heteroribbon([(5,2), (18, 2, 1), (7, 2, -1), (11, 2)], kind='armchair', vacuum=0)
g.plot(axes="xy", atoms_scale=0.6, nsc=[3, 2, 1])

pfebrer · 2023-11-08T14:46:50Z

Yes exactly, I was just afraid that the cell could follow the edge of the 18. Good! Nothing else to say then :)

tfrederiksen · 2023-11-08T14:47:10Z

You can create nanoporous graphene with arbitrary pore shapes, we should add it to the CHANGELOG 😆

The lateral fusing of nanoribbons to NPG is also how they produce this experimentally ;)

pfebrer · 2023-11-08T14:48:25Z

We are doing real science here hahah.

"Bottom-up synthesis of NPG by introduction of the vacuum argument"

zerothi · 2023-11-20T11:23:17Z

How is this coming along? Is it ready?

tfrederiksen · 2023-11-22T12:10:41Z

How is this coming along? Is it ready?

I think it's ready, no?

zerothi · 2023-11-22T13:44:14Z

Ok, I'll merge soon

github-advanced-security bot found potential problems Nov 4, 2023

View reviewed changes

zerothi requested changes Nov 4, 2023

View reviewed changes

zerothi force-pushed the cgnr branch from 256774d to 525cef3 Compare November 6, 2023 10:22

zerothi approved these changes Nov 6, 2023

View reviewed changes

zerothi approved these changes Nov 7, 2023

View reviewed changes

src/sisl/geom/flat.py Outdated Show resolved Hide resolved

src/sisl/geom/nanoribbon.py Outdated Show resolved Hide resolved

tfrederiksen added 12 commits November 8, 2023 15:06

doc: introduced 0D section and new geometries

b3bf572

simple fixes

fc3d8e7

adopted chirality for the (n, m)-index

8bcbe55

redefined ribbon cell vectors and positioning in cells

aa21049

chirality index unified and vacuum argument introduced for all ribbons

7746cf0

orthogonal cell for triangulene

1b47a56

fix test_cgnr

ca3275f

black fix

65bffa6

minor fixes

493e20a

nanoribbon atoms begin at zero along the first lattice vector (backwa…

8c16eaa

…rds compatibility)

vacuum as new parameter for heteroribbon

77dae38

final polish after rebase

e3482c5

tfrederiksen force-pushed the cgnr branch from a0ba8b5 to e3482c5 Compare November 8, 2023 14:08

added vacuum to docstring

b741ac2

zerothi merged commit 0c80505 into zerothi:main Nov 22, 2023
6 checks passed

tfrederiksen deleted the cgnr branch November 23, 2023 21:52

Chiral GNRs and [n]-triangulene #644

Chiral GNRs and [n]-triangulene #644

Conversation

tfrederiksen commented Nov 4, 2023 • edited Loading

pfebrer commented Nov 4, 2023

pfebrer commented Nov 4, 2023 • edited Loading

tfrederiksen commented Nov 4, 2023

pfebrer commented Nov 4, 2023

pfebrer commented Nov 4, 2023

zerothi left a comment

Choose a reason for hiding this comment

tfrederiksen commented Nov 4, 2023

tfrederiksen commented Nov 4, 2023

zerothi commented Nov 5, 2023

zerothi commented Nov 6, 2023

zerothi commented Nov 6, 2023

zerothi commented Nov 6, 2023

codecov bot commented Nov 6, 2023 • edited Loading

Codecov Report

pfebrer commented Nov 6, 2023

tfrederiksen commented Nov 6, 2023 • edited Loading

zerothi commented Nov 6, 2023

tfrederiksen commented Nov 6, 2023

pfebrer commented Nov 6, 2023

tfrederiksen commented Nov 6, 2023

zerothi commented Nov 6, 2023

tfrederiksen commented Nov 6, 2023 • edited Loading

tfrederiksen commented Nov 6, 2023 • edited Loading

zerothi left a comment

Choose a reason for hiding this comment

tfrederiksen commented Nov 7, 2023 • edited Loading

zerothi commented Nov 7, 2023

pfebrer commented Nov 7, 2023

pfebrer commented Nov 7, 2023

pfebrer commented Nov 8, 2023

zerothi commented Nov 8, 2023

zerothi commented Nov 8, 2023 • edited Loading

tfrederiksen commented Nov 8, 2023

tfrederiksen commented Nov 8, 2023

pfebrer commented Nov 8, 2023

pfebrer commented Nov 8, 2023

tfrederiksen commented Nov 8, 2023

pfebrer commented Nov 8, 2023

tfrederiksen commented Nov 8, 2023

pfebrer commented Nov 8, 2023

zerothi commented Nov 20, 2023

tfrederiksen commented Nov 22, 2023

zerothi commented Nov 22, 2023

tfrederiksen commented Nov 4, 2023 •

edited

Loading

pfebrer commented Nov 4, 2023 •

edited

Loading

codecov bot commented Nov 6, 2023 •

edited

Loading

tfrederiksen commented Nov 6, 2023 •

edited

Loading

tfrederiksen commented Nov 6, 2023 •

edited

Loading

tfrederiksen commented Nov 6, 2023 •

edited

Loading

tfrederiksen commented Nov 7, 2023 •

edited

Loading

zerothi commented Nov 8, 2023 •

edited

Loading