fixed pbc usage across sisl, fixes #764

zerothi / sisl

Electronic structure Python package for post analysis and large scale tight-binding DFT/NEGF calculations

https://zerothi.github.io/sisl

Mozilla Public License 2.0

182 stars 58 forks source link

fixed pbc usage across sisl, fixes #764 #767

Closed zerothi closed 4 months ago

zerothi commented 5 months ago

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!

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

pfebrer commented 5 months ago

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 commented 5 months ago

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

codecov[bot] commented 5 months ago

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 :warning:
src/sisl/_core/geometry.py	80.95%	4 Missing :warning:

Additional details and impacted files

```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zerothi commented 5 months ago

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.!