Skip to content
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

Minor Q-Chem updates #292

Merged
merged 15 commits into from
Sep 21, 2023
Merged

Minor Q-Chem updates #292

merged 15 commits into from
Sep 21, 2023

Conversation

samblau
Copy link
Contributor

@samblau samblau commented Sep 19, 2023

This PR fixes a few bugs in the Q-Chem error handlers, adds one new handler, adds some additional tests, and slightly extends post processing scratch file handling.

@samblau
Copy link
Contributor Author

samblau commented Sep 19, 2023

@janosh This is ready for review/merging. Once merged, I would appreciate it if you could release a new version. Please and thank you!

Comment on lines 165 to 166
if not os.path.exists(local_scratch):
os.mkdir(local_scratch)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not os.path.exists(local_scratch):
os.mkdir(local_scratch)
os.mkdir(local_scratch, exist_ok=True)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what @shyuep's stance on test files is for Custodian but in pymatgen we're trying to move to compressed test files to keep repo size small. Would only entail running gzip test_files/qchem/new_test_files/fileman_cpscf.qin and changing the file extension when loading the file in python.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 5.71% and project coverage change: -0.33% ⚠️

Comparison is base (85cf4aa) 66.11% compared to head (72bf741) 65.78%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
- Coverage   66.11%   65.78%   -0.33%     
==========================================
  Files          51       51              
  Lines        5610     5641      +31     
==========================================
+ Hits         3709     3711       +2     
- Misses       1901     1930      +29     
Files Changed Coverage Δ
custodian/qchem/handlers.py 8.13% <0.00%> (-0.50%) ⬇️
custodian/qchem/jobs.py 10.27% <0.00%> (-0.11%) ⬇️
custodian/qchem/tests/test_handlers.py 23.59% <14.28%> (-0.80%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@janosh janosh merged commit 4d78d09 into materialsproject:master Sep 21, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants