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

fixed pbc usage across sisl, fixes #764 #767

Merged
merged 4 commits into from
May 14, 2024
Merged

fixed pbc usage across sisl, fixes #764 #767

merged 4 commits into from
May 14, 2024

Conversation

zerothi
Copy link
Owner

@zerothi zerothi commented May 6, 2024

Now pbc usage has been moved across the
code base (I hope everything is managed).
I have added a setter for pbc to enable
fast setting pbc. It will silently ignore
any dimensions not specified.
This may be used as a shorthand for
lattice.set_boundary_condition(...)

@pfebrer could you please review this? I hope it fixes the issue!

@pfebrer
Copy link
Contributor

pfebrer commented May 6, 2024

There are some failing tests, but I don't know if they are related to this PR.

Apart from that, could we set nsc=[1,1,1] in the tests that check the modified functions? E.g. here https://github.com/zerothi/sisl/blob/43f3c2904783af2c11932058496378113ad54bf7/src/sisl/_core/tests/test_geometry.py#L1812C1-L1816C44

@zerothi
Copy link
Owner Author

zerothi commented May 6, 2024

Thanks! This actually uncovered a bit more problematic issues, I'll ask for another review once completed. Thanks!

periodic = list(periodic)
try:
periodic = map(direction, listify(periodic)) | listify
except:

Check notice

Code scanning / CodeQL

Except block handles 'BaseException' Note

Except block directly handles BaseException.
"""Allow piping of function calls"""
return self(arg)

def __getattr__(self, attr):

Check notice

Code scanning / CodeQL

Non-standard exception raised in special method Note

Function always raises
builtin-class RuntimeError
; raise AttributeError instead
Copy link

codecov bot commented May 7, 2024

Codecov Report

Attention: Patch coverage is 92.41379% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 87.28%. Comparing base (43f3c29) to head (958a1ee).

Files Patch % Lines
src/sisl/_core/lattice.py 76.66% 7 Missing ⚠️
src/sisl/_core/geometry.py 80.95% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #767      +/-   ##
==========================================
+ Coverage   87.25%   87.28%   +0.03%     
==========================================
  Files         393      393              
  Lines       50177    50264      +87     
==========================================
+ Hits        43783    43875      +92     
+ Misses       6394     6389       -5     

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

@zerothi
Copy link
Owner Author

zerothi commented May 7, 2024

Ok, it got complicated, but the pbc was never really copied over, and this might have inflected problems here and there. I think it should be done now.!

src/sisl/utils/misc.py Outdated Show resolved Hide resolved
src/sisl/utils/misc.py Show resolved Hide resolved
Now pbc usage has been moved across the
code base (I hope everything is managed).
I have added a setter for pbc to enable
fast setting pbc. It will silently ignore
any dimensions not specified.
This may be used as a shorthand for
lattice.set_boundary_condition(...)

Signed-off-by: Nick Papior <[email protected]>
Signed-off-by: Nick Papior <[email protected]>
The boundary conditions were rarely properly
shipped together with the Lattice methods.
Basically it boiled down to the copy method
which did not take all variables into account.

This commit also addes a Listify|listify
class|object that can be used to ensures
arguments are converted to a list.
It can be quite handy as it allows "piping".

Enabled a setter for pbc to fast changing
stuff. One can pass *none* to not change it.

Signed-off-by: Nick Papior <[email protected]>
@zerothi zerothi merged commit 80a8bee into main May 14, 2024
3 checks passed
@zerothi zerothi deleted the 764-nsc-vs-pbc branch May 14, 2024 07:18
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.

translate2uc should use boundary conditions (?)
2 participants