iodata
iodata copied to clipboard
Improve treatment of occuptions numbers of restricted orbitals
Fixes #158.
The following remained as it was:
- Single
MolecularOrbitals
class for all kinds of orbitals. - Restricted orbitals with integer occupation numbers are assumed to represent a restricted open- or closed-shell wavefunctions. E.g. occupation numbers
[2, 2, 1, 0, 0]
would still represent a doublet state.
The following has changed:
-
Restricted orbitals with fractional occupation numbers are assumed to be spin-paired or closed-shell, unless specified otherwise. E.g. occupation numbers
[2, 2, 1.8. 0.2, 0]
are assumed to be spin-paired. -
It is possible to deviate from the above heuristics be specifying
mo.occs_aminusb
. Just keep in mind that most file formats cannot handle this case. They essentially rely on the above heuristics. Writing a wavefunction file will normally fail whenoccs_aminusb
is set.
Codecov Report
Attention: Patch coverage is 94.62366%
with 10 lines
in your changes missing coverage. Please review.
Project coverage is 95.61%. Comparing base (
66dd4bd
) to head (a9828eb
). Report is 189 commits behind head on main.
:exclamation: Current head a9828eb differs from pull request most recent head 883d7d8
Please upload reports for the commit 883d7d8 to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## main #252 +/- ##
==========================================
- Coverage 95.64% 95.61% -0.04%
==========================================
Files 74 74
Lines 8268 8387 +119
Branches 1072 1086 +14
==========================================
+ Hits 7908 8019 +111
- Misses 165 169 +4
- Partials 195 199 +4
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@FarnazH @PaulWAyers It should be ready for review now. The few remaining coverage misses are fairly harmless.
Quality Gate failed
Failed conditions
C Reliability Rating on New Code (required ≥ A)
See analysis details on SonarCloud
Catch issues before they fail your Quality Gate with our IDE extension
SonarLint
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.
Here's the code health analysis summary for commits 7c221cb..883d7d8
. View details on DeepSource ↗.
Analysis Summary
Analyzer | Status | Summary | Link |
---|---|---|---|
✅ Success | View Check ↗ | ||
✅ Success | ❗ 4 occurences introduced 🎯 4 occurences resolved | View Check ↗ |
💡 If you’re a repository administrator, you can configure the quality gates from the settings.
This is one of the changes that should be done before 1.0.0 because it changes the API. I've brought this zombie-PR back to live by merging the main branch into this PR, so the PR is up-to-date with the recent tooling. (Deepsource complains about complexity of functions, but this problem is not due to the current PR.)
@FarnazH @PaulWAyers It would be good to have your comments on this one before it gets merged.
Thanks for the comments. I'll look up the reason for why some formats are not supporting aminusb
, if any. (If there is a good reason, it should be explained better in the comments.)
@PaulWAyers I took a closer look at the code to see how we can address your concern. In the dump_one
functions, when occs_aminusb
is set, we could convert the IOData instance from a restricted to unrestricted set of orbitals. After that conversion, everything can proceed as normal. The conversion is needed because WFX, WFN, MOLEKEL and MOLDEN formats do not support unrestricted occupation numbers with restricted orbitals. (FCHK does.)
There are a few downsides to such a conversion (at this stage):
- The resulting files contain a lot of redundant information. At the very least, there should be warning, so users are made aware of this.
- In general, we don't always do such conversion automatically, to avoid losing the original representation of the data upon writing a file. For this reason, I've created issue #191.
My suggestion would be to not add the conversion at this stage and defer it until there is a proper general solution in place as discussed in #191. (I just edited that issue for clarity.)
Thanks for the review! I'll merge this one and add a comment on the aminusb situation to #191. We have a good number of stale issues, but I love cooking with stale bread. :)