Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honeycomb flakes #636

Merged
merged 3 commits into from
Nov 2, 2023
Merged

Honeycomb flakes #636

merged 3 commits into from
Nov 2, 2023

Conversation

pfebrer
Copy link
Contributor

@pfebrer pfebrer commented Oct 31, 2023

Could a honeycomb/graphene flake geometry be useful? I'm using one currently and I think they are useful in general to create finite bilayers with arbitrary twist.

This PR is a proposal to add them.

Examples

import sisl
import sisl.viz

sisl.geom.graphene_flake(shells=3).plot(axes="xy")

newplot (4)

import sisl
import sisl.viz

sisl.geom.honeycomb_flake(shells=3, bond=1.6, atoms=["B", "N"]).plot(axes="xy")

newplot (5)

  • Closes #xxxx
  • Tests added
  • Ranned isort . at top--level
  • Documentation for functionality, docs/
  • Changes documented, CHANGELOG.md

@zerothi
Copy link
Owner

zerothi commented Oct 31, 2023

How about an API of graphene_flakes that resembles the heteroribbon? In that way attachments of various segments could be done?

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (89dcede) 87.71% compared to head (86fd8c0) 87.73%.
Report is 1 commits behind head on main.

❗ Current head 86fd8c0 differs from pull request most recent head 5c72a0a. Consider uploading reports for the commit 5c72a0a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #636      +/-   ##
==========================================
+ Coverage   87.71%   87.73%   +0.01%     
==========================================
  Files         362      362              
  Lines       48354    48403      +49     
==========================================
+ Hits        42415    42464      +49     
  Misses       5939     5939              
Files Coverage Δ
src/sisl/geom/flat.py 100.00% <100.00%> (ø)
src/sisl/geom/tests/test_geom.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

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

@pfebrer
Copy link
Contributor Author

pfebrer commented Oct 31, 2023

What would you attach to what in this case? 😅

@zerothi
Copy link
Owner

zerothi commented Oct 31, 2023

What would you attach to what in this case? 😅

A new flake/ribbon, connecting ribbons and flakes could be quite interesting ;)

@pfebrer
Copy link
Contributor Author

pfebrer commented Oct 31, 2023

Hmm yes, but the construction of the flake itself is not a composite geometry, no?

So I would imagine you can take graphene_flake and implement a section that knows how to connect to ribbons, which determines the arguments for graphene_flake and maybe rotates it, etc...

@zerothi
Copy link
Owner

zerothi commented Oct 31, 2023

yes, that sounds cool :)
Ok, but anyways, I think this is fitting

Copy link
Owner

@zerothi zerothi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great!

If adding 2 atoms, would it then alternate in each hexagon? That would be my ideal usecase, no?

Sorry, haven't tested, but looks good!

pyproject.toml Outdated Show resolved Hide resolved
src/sisl/geom/flat.py Outdated Show resolved Hide resolved
@pfebrer
Copy link
Contributor Author

pfebrer commented Oct 31, 2023

Ok, but anyways, I think this is fitting

Good 👍 I will then finish the PR some time this week probably.

To do the section stuff, it would be useful that the graphene_flake stored metadata on the Geometry, like edge_axes (the axes perpendicular to edges), edge_R (how far is the edge from the center) or edge_W (the number of C in the edge). But this requires the transformable attributes that we've discussed many times already 😅

@pfebrer
Copy link
Contributor Author

pfebrer commented Oct 31, 2023

If adding 2 atoms, would it then alternate in each hexagon? That would be my ideal usecase, no?

See the picture in the initial PR comment, is this how you expect it? Is there any other way?

@zerothi
Copy link
Owner

zerothi commented Oct 31, 2023

If adding 2 atoms, would it then alternate in each hexagon? That would be my ideal usecase, no?

See the picture in the initial PR comment, is this how you expect it? Is there any other way?

Sorry, too fast ;) Beautiful! :)

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 1, 2023

Ok, this PR is ready 👍

g = graphene_flake(shells=0, bond=1.42)
assert g.na == 6
# All atoms are close to the center
assert len(g.close(g.center(), 1.44)) == g.na

Check failure

Code scanning / CodeQL

An assert statement has a side-effect Error test

This 'assert' statement contains an
expression
which may have side effects.

g = graphene_flake(shells=1, bond=1.42)
assert g.na == 24
assert len(g.close(g.center(), 4)) == g.na

Check failure

Code scanning / CodeQL

An assert statement has a side-effect Error test

This 'assert' statement contains an
expression
which may have side effects.
src/sisl/geom/flat.py Outdated Show resolved Hide resolved
@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 1, 2023

Should I set the boundary conditions to open?

@zerothi
Copy link
Owner

zerothi commented Nov 2, 2023

Should I set the boundary conditions to open?

All you need to do is to call geometry_define_nsc(geom, [False, False, False]) then return geom.

This will set the boundary conditions correctly, to UNKNOWN.

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 2, 2023

Why do you say the correct bc is UNKNOWN and not OPEN? It is a 0D flake.

@zerothi
Copy link
Owner

zerothi commented Nov 2, 2023

Why do you say the correct bc is UNKNOWN and not OPEN? It is a 0D flake.

For this it would be better to use Dirichlet, OPEN is a reference to NEGF, perhaps OPEN should be bulk, or something. Typically OPEN is used in the nomenclature of NEGF BC.

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 2, 2023

Ok, it should be fine now 👍

src/sisl/geom/flat.py Outdated Show resolved Hide resolved
@zerothi zerothi merged commit c74de1f into zerothi:main Nov 2, 2023
2 of 5 checks passed
@tfrederiksen tfrederiksen mentioned this pull request Nov 4, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants