bug_fix | read "nelm" | compatible with vasp6 format change by Vibsteamer · Pull Request #239 · deepmodeling/dpdata

Vibsteamer · 2022-01-19T09:32:53Z

VASP6 writes NELMIN(minimum number of electronic steps) ahead of NELM(max num),
dpdata reads the first line contains "NEIM", then will take the value of NELMIN as that of NELM, inducing "false" in scf_convergence_check

this revise includes this such cases by request a stronger check that the line should not be initiated by "NELMIN" or "NELMDL".

Merge recent development on devel into master

Merge bug fixings to master

merge bug fixings to master

VASP6 writes NELMIN(minimum number of electronic steps) ahead of NELM(max num), dpdata reads the first line contains "NEIM", then will take the value of NELMIN as that of NELM, inducing "false" in scf_convergence_check this revise includes this such cases by request a stronger check that the line should not be initiated by "NELMIN" or "NELMDL"

wanghan-iapcm · 2022-01-21T01:40:07Z

-        elif 'NELM' in ii and nelm == None:
+        #find the line contains NELM but not initiated by other vasp_keys like NELMIN and NELMDL ...
+        #...format adjustmnet in vasp 6 request this check ; compatible with the older version
+        elif 'NELM' in ii and nelm == None and (ii.split()[0] != "NELMIN") and (ii.split()[0] != "NELMDL"):


I would suggest to use "NELM =" in ii, to prevent other lines with NELM appears before NELM = in future versions

the number of spaces between "NELM" and "=" might not be safe (currently maybe 2 or 3 not sure)

revise into a stricter check
expecting the following format :

"anywhere_in_the_line"+"NELM"+"any_number_of_spaces"+"="+"value(data-type-check_omitted_don't-believe-it's-needed-here)"+"whatever_else"

You may need to use regular expression.

You may need to use regular expression.

support

You may need to use regular expression.

@njzjz Could you please suggest the change with regex ?

adding a stricter check for whether the line is actually assigning value for "NELM". expecting the assignment follows the format : ``` "anywhere_in_the_line"+"NELM"+"any_number_of_spaces"+"="+"value(data-type-check_omitted_don't-believe-it's-needed-here)"+"whatever_else" ```

codecov-commenter · 2022-01-21T03:54:36Z

Codecov Report

Merging #239 (ccdd31e) into devel (a2d13e6) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            devel     #239      +/-   ##
==========================================
- Coverage   82.04%   81.99%   -0.05%     
==========================================
  Files          58       59       +1     
  Lines        5034     5121      +87     
==========================================
+ Hits         4130     4199      +69     
- Misses        904      922      +18

Impacted Files	Coverage Δ
dpdata/vasp/outcar.py	`96.29% <100.00%> (+0.03%)`	⬆️
dpdata/cp2k/output.py	`97.35% <0.00%> (-0.36%)`	⬇️
dpdata/plugins/ase.py	`65.45% <0.00%> (ø)`
dpdata/plugins/xyz.py	`100.00% <0.00%> (ø)`
dpdata/xyz/xyz.py	`100.00% <0.00%> (ø)`
dpdata/rdkit/sanitize.py	`43.77% <0.00%> (+0.06%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2d13e6...ccdd31e. Read the comment docs.

Co-authored-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>

Vibsteamer · 2022-02-09T08:26:26Z

Not sure whether the regular expression here violates the consistence in the coding style or standard, if there was supposed to be one.
Other lines are still processed without "re".

version_match error for numpy @ check of "Python package workflow run":

  Installing build dependencies: started
  Installing build dependencies: finished with status 'error'
  ERROR: Command errored out with exit status 1:
   command: /usr/share/miniconda/envs/my-rdkit-env/bin/python /usr/share/miniconda/envs/my-rdkit-env/lib/python3.6/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-3a5vw0ui/overlay --no-warn-script-location --no-binary :none: --only-binary :none: -i https://pypi.org/simple -- 'numpy>=1.20.1' 'setuptools>=43.0.0'
       cwd: None
  Complete output (2 lines):
  ERROR: Could not find a version that satisfies the requirement numpy>=1.20.1 (from versions: 1.3.0, 1.4.1, 1.5.0, 1.5.1, 1.6.0, 1.6.1, 1.6.2, 1.7.0, 1.7.1, 1.7.2, 1.8.0, 1.8.1, 1.8.2, 1.9.0, 1.9.1, 1.9.2, 1.9.3, 1.10.0.post2, 1.10.1, 1.10.2, 1.10.4, 1.11.0, 1.11.1, 1.11.2, 1.11.3, 1.12.0, 1.12.1, 1.13.0rc1, 1.13.0rc2, 1.13.0, 1.13.1, 1.13.3, 1.14.0rc1, 1.14.0, 1.14.1, 1.14.2, 1.14.3, 1.14.4, 1.14.5, 1.14.6, 1.15.0rc1, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5)
  ERROR: No matching distribution found for numpy>=1.20.1
  ----------------------------------------
ERROR: Command errored out with exit status 1: /usr/share/miniconda/envs/my-rdkit-env/bin/python /usr/share/miniconda/envs/my-rdkit-env/lib/python3.6/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-3a5vw0ui/overlay --no-warn-script-location --no-binary :none: --only-binary :none: -i https://pypi.org/simple -- 'numpy>=1.20.1' 'setuptools>=43.0.0' Check the logs for full command output.
Error: Process completed with exit code 1.

wanghan-iapcm · 2022-02-10T01:35:58Z

Not sure whether the regular expression here violates the consistence in the coding style or standard, if there was supposed to be one. Other lines are still processed without "re".

version_match error for numpy @ check of "Python package workflow run":

  Installing build dependencies: started
  Installing build dependencies: finished with status 'error'
  ERROR: Command errored out with exit status 1:
   command: /usr/share/miniconda/envs/my-rdkit-env/bin/python /usr/share/miniconda/envs/my-rdkit-env/lib/python3.6/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-3a5vw0ui/overlay --no-warn-script-location --no-binary :none: --only-binary :none: -i https://pypi.org/simple -- 'numpy>=1.20.1' 'setuptools>=43.0.0'
       cwd: None
  Complete output (2 lines):
  ERROR: Could not find a version that satisfies the requirement numpy>=1.20.1 (from versions: 1.3.0, 1.4.1, 1.5.0, 1.5.1, 1.6.0, 1.6.1, 1.6.2, 1.7.0, 1.7.1, 1.7.2, 1.8.0, 1.8.1, 1.8.2, 1.9.0, 1.9.1, 1.9.2, 1.9.3, 1.10.0.post2, 1.10.1, 1.10.2, 1.10.4, 1.11.0, 1.11.1, 1.11.2, 1.11.3, 1.12.0, 1.12.1, 1.13.0rc1, 1.13.0rc2, 1.13.0, 1.13.1, 1.13.3, 1.14.0rc1, 1.14.0, 1.14.1, 1.14.2, 1.14.3, 1.14.4, 1.14.5, 1.14.6, 1.15.0rc1, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5)
  ERROR: No matching distribution found for numpy>=1.20.1
  ----------------------------------------
ERROR: Command errored out with exit status 1: /usr/share/miniconda/envs/my-rdkit-env/bin/python /usr/share/miniconda/envs/my-rdkit-env/lib/python3.6/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-3a5vw0ui/overlay --no-warn-script-location --no-binary :none: --only-binary :none: -i https://pypi.org/simple -- 'numpy>=1.20.1' 'setuptools>=43.0.0' Check the logs for full command output.
Error: Process completed with exit code 1.

I do not understand why it is introduce. @njzjz any idea?

njzjz · 2022-02-10T01:39:42Z

I do not understand why it is introduce. @njzjz any idea?

Numpy 1.20 drops support for Python 3.6. It's unclear to me which package needs numpy>=1.20.1, maybe pymatgen?

wanghan-iapcm · 2022-02-10T01:45:57Z

I do not understand why it is introduce. @njzjz any idea?

Numpy 1.20 drops support for Python 3.6. It's unclear to me which package needs numpy>=1.20.1, maybe pymatgen?

Our UT uses pymatgen versions 2019.1.13, 2019.7.30, so the restriction should not be introduced by them

njzjz · 2022-02-10T03:25:19Z

I do not understand why it is introduce. @njzjz any idea?

Numpy 1.20 drops support for Python 3.6. It's unclear to me which package needs numpy>=1.20.1, maybe pymatgen?

Our UT uses pymatgen versions 2019.1.13, 2019.7.30, so the restriction should not be introduced by them

I don't see any version constraint?

dpdata/.github/workflows/test.yml

Line 26 in 3edbc24

    
           run: source $CONDA/bin/activate my-rdkit-env && pip install .[amber,ase,pymatgen] coverage codecov

I think what you mean is dpgen

wanghan-iapcm · 2022-02-10T05:11:04Z

I do not understand why it is introduce. @njzjz any idea?

Numpy 1.20 drops support for Python 3.6. It's unclear to me which package needs numpy>=1.20.1, maybe pymatgen?

Our UT uses pymatgen versions 2019.1.13, 2019.7.30, so the restriction should not be introduced by them

I don't see any version constraint?

dpdata/.github/workflows/test.yml

Line 26 in 3edbc24

run: source $CONDA/bin/activate my-rdkit-env && pip install .[amber,ase,pymatgen] coverage codecov

I think what you mean is dpgen

You are right I took dpgen. Could you please update our test? I suggest we do not support python 3.6 any longer.

wanghan-iapcm and others added 6 commits November 15, 2021 07:45

Merge pull request deepmodeling#216 from amcadmus/master

1d1084d

Merge recent development on devel into master

Merge branch 'master' into devel

6569780

Merge pull request deepmodeling#221 from amcadmus/master

f242cbe

Merge bug fixings to master

Merge remote-tracking branch 'upstream/devel' into devel

c377f85

Merge pull request deepmodeling#228 from amcadmus/master

306172d

merge bug fixings to master

wanghan-iapcm requested a review from AnguseZhang January 19, 2022 23:39

wanghan-iapcm approved these changes Jan 19, 2022

View reviewed changes

wanghan-iapcm reviewed Jan 21, 2022

View reviewed changes

wanghan-iapcm mentioned this pull request Feb 9, 2022

Dpdata is not compatible with vasp6.3 #241

Closed

njzjz reviewed Feb 9, 2022

View reviewed changes

Comment thread dpdata/vasp/outcar.py Outdated

Vibsteamer and others added 2 commits February 9, 2022 15:58

Apply suggestions from code review

13968cb

Co-authored-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>

Apply suggestions from code review

68d9578

Co-authored-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>

Vibsteamer requested a review from wanghan-iapcm February 9, 2022 08:42

wanghan-iapcm approved these changes Feb 10, 2022

View reviewed changes

wanghan-iapcm merged commit c08c74c into deepmodeling:devel Feb 10, 2022

njzjz mentioned this pull request Feb 10, 2022

bump test python version to 3.7-3.9 #244

Merged

Uh oh!

Conversation

Vibsteamer commented Jan 19, 2022

Uh oh!

wanghan-iapcm Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

Vibsteamer Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

njzjz Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

wanghan-iapcm Jan 22, 2022

Choose a reason for hiding this comment

Uh oh!

amcadmus Feb 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Vibsteamer commented Feb 9, 2022

Uh oh!

wanghan-iapcm commented Feb 10, 2022

Uh oh!

njzjz commented Feb 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wanghan-iapcm commented Feb 10, 2022

Uh oh!

njzjz commented Feb 10, 2022

Uh oh!

wanghan-iapcm commented Feb 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

amcadmus Feb 9, 2022 •

edited

Loading

codecov-commenter commented Jan 21, 2022 •

edited

Loading

njzjz commented Feb 10, 2022 •

edited

Loading