Skip to content
This repository has been archived by the owner on Feb 12, 2021. It is now read-only.

Version 0.2 #12

Merged
merged 47 commits into from
Apr 20, 2020
Merged

Version 0.2 #12

merged 47 commits into from
Apr 20, 2020

Conversation

hitliaomq
Copy link
Collaborator

Add dfttk run [options] and dfttk config [options] command
Add some test
Enhance documents
Change the change_log

@hitliaomq hitliaomq requested a review from bocklund March 30, 2020 07:46
Missed a comma
Drop test for python3.5 and add python3.7 and python3.8
py.test -v tests/  ->  pytest -v
Fix the test by specify the test file (start with test_)
@bocklund
Copy link
Member

I see the tests are passing now, I'll try to look at this today

README.rst Outdated Show resolved Hide resolved
dfttk/EVcheck_QHA.py Outdated Show resolved Hide resolved
dfttk/EVcheck_QHA.py Show resolved Hide resolved
dfttk/scripts/config_dfttk.py Outdated Show resolved Hide resolved
dfttk/scripts/config_dfttk.py Outdated Show resolved Hide resolved
dfttk/scripts/config_dfttk.py Outdated Show resolved Hide resolved
dfttk/scripts/config_dfttk.py Outdated Show resolved Hide resolved
dfttk/scripts/run_dfttk.py Outdated Show resolved Hide resolved
dfttk/scripts/run_dfttk.py Outdated Show resolved Hide resolved
@@ -71,6 +71,12 @@ def get_wf_gibbs(structure, num_deformations=7, deformation_fraction=(-0.1, 0.1)
vasp_cmd = vasp_cmd or VASP_CMD
db_file = db_file or DB_FILE

if db_file == ">>db_file<<":
Copy link
Member

Choose a reason for hiding this comment

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

Atomate’s envchk should be used to do the lookup

Copy link
Collaborator Author

@hitliaomq hitliaomq Apr 18, 2020

Choose a reason for hiding this comment

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

@bocklund In some functions wrote by Peng Gao, some check for the database is done out of the firetask or fireworks, so the db_file can't be touched by env_chk.

What should I do?

Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't have any code that's part of a Workflow that accesses the database outside of a Firework, so that means check_relax_path should be a Firetask. In addition, check_relax_path assumes that you have access to the filesystem that the relaxation was run on (not true if you are using multiple compute resources).

Since we're going to update the way the relaxation scheme works soon, don't worry about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK. Thank you a lot.

I will check the code, and move the function using db_file to Firetask in the next version.

Modification according to Brandon's comments
Modification according to Brandon's comments
@bocklund
Copy link
Member

Changes look good to me and the PR can be merged when you are ready @hitliaomq

@hitliaomq hitliaomq merged commit 77c7ab9 into PhasesResearchLab:master Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants