Growth direction of zigzag heteroribbons

[tfrederiksen]

The growth direction for heteroribbons should be the x-axis also for kind=zigzag. This PR produces expected structures, eg.:

import sisl
import sisl.viz
g = sisl.geom.graphene_heteroribbon([(5, 2), (6, 3)], kind='zigzag')
g.plot(axes="xy", atoms_scale=0.6, nsc=[6, 2, 1])

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

[zerothi]

I don't know if that is intended, but it creates periodic defects, and also the 4 square atoms has a non-conforming bond-length?
What should it do, this? Or?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5339e68) 87.65% compared to head (9d9fc1d) 87.66%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #648   +/-   ##
=======================================
  Coverage   87.65%   87.66%           
=======================================
  Files         364      364           
  Lines       48520    48518    -2     
=======================================
+ Hits        42531    42532    +1     
+ Misses       5989     5986    -3     

Files	Coverage Δ
src/sisl/geom/nanoribbon.py	91.46% <100.00%> (+0.73%)	⬆️

... and 1 file with indirect coverage changes

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

[tfrederiksen]

You're right! My example was not good, here is a better one:

g = sisl.geom.graphene_heteroribbon([(5, 6), (7, 3), 5], kind='zigzag')
g.plot(axes="xy", atoms_scale=0.6, nsc=[2, 1, 1])

But yes, more generally the zigzag method seems a bit limited to produce desired structures. @pfebrer, how would do you for instance attach an extra hexagon to the edge of a zigzag ribbon? The following does not generate what I have in mind:

g = sisl.geom.graphene_heteroribbon([(5, 6), (6, 3)], kind='zigzag')
g.plot(axes="xy", atoms_scale=0.6, nsc=[2, 1, 1])

[pfebrer]

Hmm this last mistaken structure happens only after your commit or it also happens before the commit?

I guess there was a reason I set the growth axis different, maybe it doesn't work if you change it. If it worked before I guess we could rotate at the end when all coordinates are determined.

[pfebrer]

But in this last example if you set the junction at the center properly the one at the boundary will be wrong, so there's really no right solution, no? Still I think when appending the second section the code doesn't know that it is the last one, so it's probably still a bug.

[tfrederiksen]

Hmm this last mistaken structure happens only after your commit or it also happens before the commit?

Before this commit there was no heteroribbon. The segments were stacked vertically with vacuum gaps in between, for instance:

g = sisl.geom.graphene_heteroribbon([(5, 2), (7, 2)], kind='zigzag')
g.plot(axes="xy", atoms_scale=0.6, nsc=[4, 1, 1])

[pfebrer]

This is very strange, has the growth axis changed in nanoribbon?

[tfrederiksen]

This is very strange, has the growth axis changed in nanoribbon?

All I can think of is the swap of graphene lattice vectors in #489.

[zerothi]

So I guess the heteroribbon tests did not catch the change of the axis order, what should I do here? Let me know when I should merge!

[pfebrer]

Hmm I guess we could merge this and fix the edge cases later. It is better to have something that produces good ribbons sometimes than something that creates parallel ribbons 😅

[zerothi]

when @tfrederiksen acknowledges, I'll merge, thanks!

[tfrederiksen]

Green light from my side to merge. Should we open an issue with the aim to address the zigzag heteroribbons more generally later?

[zerothi]

Ok, i'll merge