Make custodian threadsafe (explicit file paths)

[zulissimeta]

Custodian is great, but the expectation is that it's running in its own process and the current working directory is used everywhere. Most file IO is hard-coded to either "./", os.getcwd(), or just implicitly assumes one of those. This could be a problem if multiple threads (say in a workflow) were running custodian jobs, since all threads share the same cwd.

This PR is a first stab at making paths explicitly everywhere so the process cwd is never used implicitly or explicitly. Only the VASP paths have been updated so far.

[Andrew-S-Rosen]

Thank you very much for this PR! I genuinely think this is a very important one to address, and I would be happy to help get this over the finish line.

Some things to do:

• Some minor refactoring (e.g. there are some repeated os.path.join calls that we can define once and be done with)
• Some simple tests
• Get all checks to pass

Note: Addresses #316 for VASP.

[shyuep]

Nice. I agree it is needed. Never realized that the relative paths are non-threadsafe.

[zulissimeta]

@shyuep It's not obvious to me how to update _mod_input in the nwchem handlers. I don't see it actually used/referenced anywhere; can this be removed?

https://github.com/materialsproject/custodian/blob/master/custodian/nwchem/handlers.py#L42

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 80.78947% with 73 lines in your changes are missing coverage. Please review.

Project coverage is 62.78%. Comparing base (9b67478) to head (fdf2ab1).

Files	Patch %	Lines
custodian/vasp/jobs.py	70.14%	20 Missing ⚠️
custodian/custodian.py	72.09%	12 Missing ⚠️
custodian/cp2k/validators.py	0.00%	10 Missing ⚠️
custodian/vasp/handlers.py	94.11%	7 Missing ⚠️
custodian/nwchem/jobs.py	0.00%	5 Missing ⚠️
custodian/cp2k/jobs.py	69.23%	4 Missing ⚠️
custodian/feff/handlers.py	66.66%	4 Missing ⚠️
custodian/cp2k/handlers.py	86.95%	3 Missing ⚠️
custodian/feff/jobs.py	80.00%	2 Missing ⚠️
custodian/lobster/jobs.py	86.66%	2 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
+ Coverage   62.67%   62.78%   +0.11%     
==========================================
  Files          35       35              
  Lines        3003     3018      +15     
==========================================
+ Hits         1882     1895      +13     
- Misses       1121     1123       +2     

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

[shyuep]

@zulissimeta Pls go ahead and delete it. If the tests pass, I have no problem. My philosophy is - if the original author didn't bother to write tests to guarantee a functionality, no one else has a responsibility to ensure that it sticks around.

Also, feel free to just focus on the classes that you actually use (e.g., vasp). There is no need to concern yourself with making other components threadsafe (e.g., nwchem or qchem) unless you are using them or are inclined to do it.

[shyuep]

Thanks for all the work!

[zulissimeta]

Thanks @shyuep - you might want to leave this open for a little bit; I think @Andrew-S-Rosen was going to take a pass and add some simple tests for the directory functionality.

[Andrew-S-Rosen]

Thanks @shyuep and @zulissimeta! I'll open a follow-up PR for additional tests and any fixes.