Stop periodic systems from crashing System construction - #50
Merged
Conversation
System.__init__ accepts cell as Cell|ndarray|None but passed it straight to spacegroup_number/spacegroup, which are beartype-hinted cell: Cell. read_xyz and read_gen return raw ndarrays for periodic input, so with beartype active every non-vacuum system aborted construction. Compute the spacegroup only when the cell is an actual Cell and otherwise degrade to None, since the spacegroup is unused by the gas-phase RRHO calculation. Also fix the spacegroup_number property, which returned _spacegroup instead of _spacegroup_number.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
+ Coverage 93.63% 93.80% +0.16%
==========================================
Files 21 21
Lines 1194 1194
==========================================
+ Hits 1118 1120 +2
+ Misses 76 74 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #48.
System.__init__acceptscell: Cell | np.ndarray | Nonebut passed the value straight tospacegroup_number/spacegroup, which are beartype-hintedcell: Cell. Sinceread_xyz/read_genreturn raw ndarrays for periodic input (andbeartype_this_packageis active), every non-vacuum system aborted construction withBeartypeCallHintParamViolation— the package was effectively vacuum-only, and the repo's own periodic fixturetests/data/thermo/geo_opt.gencould not be run.Fix
Cell; otherwise degrade tospacegroup=None. The cell/spacegroup is unused by the gas-phase RRHO calculation, so periodic input now constructs cleanly instead of crashing.spacegroup_numberproperty, which returned_spacegroup(the symbol string) instead of_spacegroup_number.Tests
spacegroup_number/spacegroupasNone(previously raised).spacegroup_number/spacegroupto their respective attributes.141 passed, 0 skipped (suite run with the DFTB+ binaries enabled); pylint 7.94/10.