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

Small change to gunzip to allow better restarting #476

Merged
merged 6 commits into from
Aug 17, 2023

Conversation

jmmshn
Copy link
Contributor

@jmmshn jmmshn commented Aug 15, 2023

Small change to gunzip to allow better restarting

Proposed change to make resuming of long-running calculations easier

If we want to resume a calculation
VaspJob in custodian has an auto_continue argument. This makes sure that CONTCAR is copied to POSCAR whenever possible.
https://github.com/materialsproject/custodian/blame/26bf28a4b15b1253c955f3f8d6bd427bbc398e3a/custodian/vasp/jobs.py#L209-L229

So setting something like this at job creation or setting it in a restart script/CLI will be useful.
For a given maker:

BaseVaspMaker.run_vasp_kwargs = {"vasp_job_kwargs" : {"auto_continue" : True}}

However, most of the VASP jobs in atomate2 will copy from prev_dir first and thus overwrite the current state of the directory. In most cases, this will just error out because force_overwrite=False by default.
For proper restarting: the copy_vasp_files function should make sure it does not overwrite existing files.
This is currently not possible. This PR changes the gunzip function to accept force="skip" as an argument.

@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #476 (9f5c41f) into main (18d014a) will decrease coverage by 0.19%.
The diff coverage is 27.77%.

❗ Current head 9f5c41f differs from pull request most recent head c8eb162. Consider uploading reports for the commit c8eb162 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
- Coverage   65.71%   65.52%   -0.19%     
==========================================
  Files          78       78              
  Lines        7462     7476      +14     
  Branches      964      970       +6     
==========================================
- Hits         4904     4899       -5     
- Misses       2255     2275      +20     
+ Partials      303      302       -1     
Files Changed Coverage Δ
src/atomate2/vasp/files.py 87.71% <ø> (ø)
src/atomate2/utils/file_client.py 43.22% <27.77%> (-0.60%) ⬇️

... and 2 files with indirect coverage changes

@utf
Copy link
Member

utf commented Aug 17, 2023

I just made some edits to add the same feature to gzip files as well, to make sure the APIs are consistent. Would you be able to add a corresponding test for the force mode for gzipping files?

I also butchered the linting, sorry about that.

@utf
Copy link
Member

utf commented Aug 17, 2023

Actually, nvm. This looks good!

@utf utf marked this pull request as ready for review August 17, 2023 15:28
@utf utf merged commit c9e67d0 into materialsproject:main Aug 17, 2023
4 of 5 checks passed
@utf utf added the enhancement Improvements to existing features label Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants