Repository navigation
Add option to disable saving in run_pyscf - #73
Mounika-2604 wants to merge 2 commits into
Conversation
|
Hi @mhucka , I’ve implemented the requested save option for run_pyscf(), keeping the default behavior unchanged. I also added a regression test for save=False, and all tests pass (2 passed, 1 warning). The changes are pushed to my fix-issue-56 branch. |
mhucka
left a comment
mhucka
left a comment
There was a problem hiding this comment.
Thanks for this contribution. Some change requests follow.
| verbose=False): | ||
| verbose=False, | ||
| save=True): | ||
|
|
There was a problem hiding this comment.
Please delete the extra blank line.
| """ | ||
| This function runs a pyscf calculation. |
There was a problem hiding this comment.
The first line of a docstring should start right after the triple quotes.
| """ | |
| This function runs a pyscf calculation. | |
| """This function runs a pyscf calculation. |
| save=False) | ||
|
|
||
| assert isinstance(new_mole, PyscfMolecularData) | ||
| assert not save_called No newline at end of file |
There was a problem hiding this comment.
Make sure the file ends with a newline.
| run_fci=True, | ||
| verbose=1) | ||
| assert isinstance(new_mole, PyscfMolecularData) | ||
|
|
There was a problem hiding this comment.
Two blank lines between functions at the top level of a file.
| def test_run_pyscf_without_save(monkeypatch): | ||
| save_called = [] | ||
|
|
||
| def mock_save(self): | ||
| save_called.append(True) | ||
|
|
||
| monkeypatch.setattr(PyscfMolecularData, 'save', mock_save) | ||
|
|
||
| new_mole = run_pyscf(molecule, | ||
| run_scf=True, | ||
| save=False) | ||
|
|
There was a problem hiding this comment.
This code looks like it was generated by an LLM. They are fond of using mocks and monkey patching, but it's usually unnecessary. In this particular case, MolecularData accepts a data_directory (or filename) in its constructor and saves to f"{self.filename}.hdf5". Using pytest's built-in tmp_path fixture (or tempfile.TemporaryDirectory), the test can be rewritten to check whether the .hdf5 file is actually created on disk rather than monkey-patching PyscfMolecularData.save.
|
Thanks for the thorough review and helpful suggestions, @mhucka! I have updated the PR to address all the feedback:
Please let me know if any further adjustments are needed! |
2 participants
Summary
Fixes #56.
This PR adds a
saveoption torun_pyscf()to allow callers to disablesaving the calculated molecular data to disk.
Changes
save=Trueparameter torun_pyscf().save=True.Testing
pytest openfermionpyscf/tests/_run_pyscf_test.py -v