-
Notifications
You must be signed in to change notification settings - Fork 8
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
Added to_cisdtq functionality to RFCI wave function types for extract… #69
Conversation
…All seems to be working correctly. Tests and examples to come.
…er than ndarray. Rather than get from HDF5 file, just use try/except block for the time being, and fix properly later...(presume whether it stores as ndarray or hdf5 will depend on what it thinks is available memory).
type of correction == 'external-ccsdv', where the parent Coulomb integral is contracted. | ||
If not passed in, MO energy if needed will be constructed from the diagonal of | ||
get_fock() of embedding base class, and the eris will be also be obtained from the | ||
embedding base class. Optional. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we not always just use the embedding class attributes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my question was why bother with eris.mo_energy
at all, and not always just get the MO energies from the embedding class - actually, you can get them also from the fragment, with the function get_fragment_mo_energy
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the eri's themselves are potentially used here too, when we are contracting with the low-level coulomb integrals. I think for solids, there may be differences in MO energies regarding whether they are the shifted ones or not?
@obackhouse I added a few comments, but looks good (and like a lot of work!) well done |
Hey all, it seems like a general consensus has been reached that all functionality in here is stable and ready to go, so unless there are any complaints (or anyone would like to do it before me) I'm happy to merge it into #65 branch tomorrow. I've resolved any conflicts in this merge already (these were just the changes to the CI triggering for PRs not into master or dev, and both branches implementing the same fix for the pyscf symmetry operation renaming). We can then move on to merging that branch into master, which I'm hoping to be relatively straightforward- though we should probably add these changes into the changelog for that version before merger if we want to include them in that release. Would @abhishekkhedkar09 or @obackhouse be happy to write that? |
oh and the changelog can just go in the release tag? |
I've also just noticed my conflict resolution commit has triggered possibly the first CI test run on this branch, since they didn't run after your fix for some reason, so we might have some fixes to make there as well! |
As in we don't need to do it until actually the commit where we release v1.0.2? Yeah, I just meant we shouldn't forget to do that if we're sticking to version-specific changelogs. I'm assuming release would correspond to merging #65 into master? |
Even further, usually you merge into features straight into master (rather than via a |
Yeah, I don't disagree- I'd say lets stick with merging straight to master in future. I've done some tracing down on the test failures here, but am going to open a new PR to avoid too much discussion here since it mainly concerns the spin-symmetry-broken PR that was merged in here a while back. |
…ured TestMolecule.uhf_stable uses symmetry-broken reference.
Now, there is just an 'external' keyword for the external correction. Instead, whether the interaction in the T3 * V term is contracted with V in the high-level (i.e. FCI) 'smaller' space, or the low-level (i.e. CCSD) 'larger' space, is controlled via an optional argument to the 'externally_correct' function. This argument is 'low_level_coul', and defaults to 'True'. This means that the default behaviour is equivalent to the 'external-ccsdv' correction previously. Tests and examples updated.
with translationally equivalent fragments.
Lets merge this, once we are sure tests pass. |
Fix dissociated H2 test.
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## v1.0.2a #69 +/- ##
===========================================
+ Coverage 69.29% 71.56% +2.27%
===========================================
Files 131 133 +2
Lines 17395 18439 +1044
Branches 2513 2570 +57
===========================================
+ Hits 12053 13195 +1142
+ Misses 4612 4506 -106
- Partials 730 738 +8
... and 5 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
…ion of C1 -> C4 info