Dataclass for solver kwargs #77
Merged
Conversation
This file contains 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
- this fixed a likely bug in optimize.
previously we had
J0 = [[J0[-1, -1]]]
now it is
J0 = [[J0[-1][-1]]]
since it is a list[list[float]]
finished BEOPT attrs refactoring
call self.scratch_dir.cleanup in both `optimize` and `oneshot`
makes much more sense like this
and fixed corresponding tests
Merged
lweisburn
approved these changes
Jan 10, 2025
There was a problem hiding this comment.
This looks good to me! We (I) should work on setting up some SCI examples and tests to test out the code science-wise and think about the default parameters, but otherwise it all makes sense and makes the code more legible. We should confirm with @ShaunWeatherly to make sure the DMRG changes work for him
| from quemb.shared.typing import KwargDict | ||
|
|
||
|
|
||
| class UserSolverArgs(ABC): |
There was a problem hiding this comment.
I feel like this might belong to shared.typing, although it would need a short docstring. I think it's general enough that it may find use in other modules (also it is used in both the pbe/molbe modules).
There was a problem hiding this comment.
Also, I'm wondering why we have a base class for UserSolverArgs but not for SolverArgs. Not saying we need one necessarily, I'm just unsure why it's structured this way.
There was a problem hiding this comment.
The final processed DMRG_Args and SHCI_Args have no relation anymore and are independent types.
I introduced the abstract type only for the user arguments to not bloat the interface of BE.
One could do this even without inheritance by introducing a union type DMRG_ArgsUser | SHCI_ArgsUser.
ShaunWeatherly
approved these changes
Jan 10, 2025
ShaunWeatherly
added a commit
that referenced
this pull request
Jan 14, 2025
* Automatic generation of API documentation (#70) - Create API reference automatically for all public members of quemb Before this change one had to manually create rst files and manage them. Now they are recursively created automatically. The layout of the documentation matches the layout of the namespace - This change automatically ensures that all (public) docstrings are actually parsed by Sphinx and it is ensured that they are properly written. Had to fix some docstrings - Type-hinting is now picked up by sphinx. * H5py use contextmanager (#73) * put if __main__ guard to test * use h5py contextmanager nearly everywhere * fixed a bug from wrong close statement * Fix parallelization and add parallel tests (#75) * fix _opt to work with be_parallel * modify oneshot to work with be_func_parallel * modify two tests to use nproc > 1 * fix ruff errors * modify octane test * Remove CCSD reference from octane test * Final scratch dir attempt (#74) # Mayor changes This PR unifies the use of the ScratchDir. It is now guaranteed, that files are cleaned up if the calculation exits with no error. (If the user passes `WorkDir(cleanup_at_end=False)` the files are retained). A `WorkDir` is passed from the top of the call stack to the bottom. This allows the user to change the WorkDir if necessary. (Fixes #19) Got rid of some special cased keyword arguments for DMRG and moved them into an explicit `DMRG_solver_kwargs`. This way we can catch arguments that are supplied and do nothing. It is probably a good idea to do the same for SHCI specific keywords @lweisburn . It is probably a good idea to get rid of the dictionary altogether and to replace it with a dataclass. # Minor changes If the `__init__` of a class was just populating the attributes, then I replaced the class definition with `attrs.define`. On the way I did a lot of type hinting. It is probably a good idea to double check my assumptions in review. Additionally, I did some refactoring, where explicit open-close pairs where replaced with context managers. The same was true for multiprocessing Pools. Here and there I encountered constructs that could be simplified a lot ```python3 veffs = [] [veffs.append(result.get()) for result in results] # becomes veffs = [result.get() for result in results] ``` or ```python3 self.frozen_core = False if not fobj.frozen_core else True # becomes self.frozen_core = fobj.frozen_core ``` or ```python3 rdm_return = False if relax_density: rdm_return = True # becomes rdm_return = relax_density ``` * Update fragment energies (#79) * Untangling eeval and frag_energy energy evaluation options * small changes for frag_energy consistency * start changing eeval and ereturn in parallel functions, making serial and parallel consistent * put if __main__ guard to test * use h5py contextmanager nearly everywhere * changed mbe.StoreBE into attrs defined class * ignore broken link to numpy.float64 in documentation * fixed a bug from wrong close statement * testsuite don't wait for analysis * added types for molbe BE object * fixed wrong close statements * fixed scratch-dir for molbe-be * added typing for molbe.BE.optimize - this fixed a likely bug in optimize. previously we had J0 = [[J0[-1, -1]]] now it is J0 = [[J0[-1][-1]]] since it is a list[list[float]] * renamed _opt.py to opt.py finished BEOPT attrs refactoring * pass on solver_kwargs to be_func * added types to be_func * added delete multiple_files function * use the new scratch dir in be_func * added types to molbe.BE.optimize + call self.scratch_dir.cleanup call self.scratch_dir.cleanup in both `optimize` and `oneshot` * moved schmidt_decomposition to prevent circular import makes much more sense like this * added type hints to be_func_parallel * fixed typo * fixed several small errors in the code * fixed be_func_parallel calls * simplified be_func_parallel * added types to run_solver * use frag_scratch in be_func_parallel * use frag_scratch in run_solver * added typehints to kbe BE class * simplified a few boolean expressions * the tests should be working now * ensure scratch cleanup for kbe * removed call to self.compute_energy_full (related to #35) * write nicer Pool statements - used proper list comprehensions - use contextmanager for Pool * fixed naming error * use more explicit way of freeing memory (it is still ugly... ) * refactored expression * use Tuple[int, ...] for numpy shapes :-( * refactored WorkDir to use atexit.register for cleanup * added better docstring for config * require static analysis to be ok for running test suite * renamed DMRG specific solver kwargs to its proper name * better naming for DMRG stuff * added types to block2 DMRG function * refactor some DMRG arguments * change behaviour of scratch contextmanager now it is ensured, that files are deleted even after an exception when using it as context manager and cleanup_at_end=True * fixed the deadlock in the test requirements * added new scratch dir also to ube * avoid list[list[float]]; use consistently array instead * Update energy keywords and logic in restricted BE, serial and parallel, oneshot and parallel. Update and rearrange some documentation for the keywords TODO: add non-cumulant energy, update unrestricted BE * Update kbe/pbe and kbe/misc for consistency * Fix kbe/pbe and kbe/misc to raise error for non-cumulant * Add non-cumulant energy option for molecular code * remove mypy attempt from get_energy_frag for now * fix be2puffin call of oneshot and update ube oneshot default * Update get_frag_energy function to work for periodic calculations * Remove double del line * remove redundant frag_energy keyword * Update src/quemb/molbe/helper.py, eri_file to be optional Co-authored-by: Minsik <[email protected]> * Update src/quemb/molbe/mbe.py: ebe_tot readability Co-authored-by: Minsik <[email protected]> * move use_cumulant to optimize and oneshot, not BE --------- Co-authored-by: Leah Weisburn <[email protected]> Co-authored-by: Oskar Weser <[email protected]> Co-authored-by: Minsik <[email protected]> * Adding back veff0 for molecular code, removing the now-unnecessary hf_veff from be_func (#81) * More type annotations + simpler boolean expressions (#76) Simplifications in the code - A couple more type annotations - simpler boolean expressions - removed keyword arguments if they are always true in the code * Dataclass for solver kwargs (#77) - Introduced a dataclass for solver kwargs. This is makes it much more explicit what arguments are supported by DMRG and SHCI - Could simplify the interface of BE by removing a couple of arguments * Simplify init (#82) - moved the saving operation out of the __init__ method. - don't redundantly store information from `fobj`, but use `fobj` itself directly. * Make numpy shorter (#83) - replacing `np.dot` with `@` - replacing the `numpy.something` calls with `from numpy import something` if functions appear several times - replacing the `numpy.something` call with `np.something` if a function appears rarely --------- Co-authored-by: Oskar Weser <[email protected]> Co-authored-by: Leah Weisburn <[email protected]> Co-authored-by: Leah Weisburn <[email protected]> Co-authored-by: Minsik <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Introduced a dataclass for solver kwargs. This is makes it much more explicit what arguments are supported by DMRG and SHCI
Could simplify the interface of BE by removing a couple of arguments