Calypso #663

wangzyphysics · 2022-03-10T02:54:59Z

reorganize code structure
add unitteset
give some necessary explainations about new keys
fix pbs script of gpu : from '#PBS -l nodes=%d:ppn=%d:gpus=%d\n' to '#PBS -l select=%d:ncpus=%d:ngpus=%d\n'

Merge devel into master

fix typo in github actions release to conda

Change-Id: I17774bee345634e4e72bd783e8112eefaaf9f0d3 Co-authored-by: Zhengju Sha <[email protected]>

Merge recent devel into master

Merge 'calypso' from origin into 'calypso'

codecov-commenter · 2022-03-13T10:45:28Z

Codecov Report

Merging #663 (43ef866) into devel (6de10d0) will increase coverage by 0.13%.
The diff coverage is 35.93%.

@@            Coverage Diff             @@
##            devel     #663      +/-   ##
==========================================
+ Coverage   33.16%   33.30%   +0.13%     
==========================================
  Files          86       92       +6     
  Lines       14837    16270    +1433     
==========================================
+ Hits         4921     5418     +497     
- Misses       9916    10852     +936

Impacted Files	Coverage Δ
dpgen/auto_test/EOS.py	`64.70% <0.00%> (ø)`
dpgen/data/tools/bcc.py	`21.05% <ø> (ø)`
dpgen/data/tools/create_random_disturb.py	`0.00% <0.00%> (ø)`
dpgen/data/tools/diamond.py	`20.00% <ø> (ø)`
dpgen/data/tools/fcc.py	`100.00% <ø> (ø)`
dpgen/data/tools/hcp.py	`20.00% <ø> (ø)`
dpgen/data/tools/sc.py	`22.22% <ø> (ø)`
dpgen/generator/lib/calypso_check_outcar.py	`0.00% <0.00%> (ø)`
dpgen/generator/lib/calypso_run_model_devi.py	`0.00% <0.00%> (ø)`
dpgen/generator/lib/calypso_run_opt.py	`0.00% <0.00%> (ø)`
... and 15 more

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 6de10d0...43ef866. Read the comment docs.

AnguseZhang · 2022-03-14T15:19:48Z

@amcadmus Should I start the code review?

wanghan-iapcm · 2022-03-15T03:05:24Z

@amcadmus Should I start the code review?

No. I have personally contacted the contributor to clarify how to improve the PR.

amcadmus · 2022-03-20T03:44:48Z

dpgen/dispatcher/PBS.py

@@ -94,7 +94,7 @@ def sub_script_head(self, res):
        if res['numb_gpu'] == 0:
            ret += '#PBS -l nodes=%d:ppn=%d\n' % (res['numb_node'], res['task_per_node'])
        else :
-            ret += '#PBS -l nodes=%d:ppn=%d:gpus=%d\n' % (res['numb_node'], res['task_per_node'], res['numb_gpu'])
+            ret += '#PBS -l select=%d:ncpus=%d:ngpus=%d\n' % (res['numb_node'], res['task_per_node'], res['numb_gpu'])


I would not recommend change the PBS resource list here. Moreover, it is better to use deepmodeling/dpdispatcher package instead of the dispatcher in dpgen.

amcadmus · 2022-03-20T03:46:58Z

dpgen/generator/lib/modd_calypso.py

+    f.close()
+    os.chdir(cwd)
+
+def Analysis(iter_index,jdata,calypso_run_opt_path,calypso_model_devi_path):


function names should be in lower case. Analysis -> analysis

amcadmus · 2022-03-20T03:47:27Z

dpgen/generator/lib/modd_calypso.py

+calypso_run_opt_name = 'gen_stru_analy'
+calypso_model_devi_name = 'model_devi_results'
+
+def GenStructures(iter_index,jdata,mdata):


function names should be in lower case. GenStructures -> gen_structures

amcadmus · 2022-03-20T03:48:49Z

dpgen/generator/lib/modd_calypso.py

@@ -0,0 +1,424 @@
+"""
+calypso model devi:


I would recommend changing the name of the file to "model_devi_calypso.py", so the name is self-explanatory. modd is not a standard abbreviation in dpgen.

amcadmus · 2022-03-20T03:50:38Z

dpgen/generator/lib/modd.py

+    return devi
+
+
+def Modd(all_models,type_map):


function names should be in lower case. Modd -> model_devi

amcadmus · 2022-03-20T04:27:29Z

dpgen/generator/lib/modd.py

+    return devi
+
+
+def Modd(all_models,type_map):


It seems that Modd only works for the case of calypso. Please move it to module model_devi_calypso

amcadmus · 2022-03-20T04:29:43Z

dpgen/generator/run.py

+        if model_devi_engine != 'calypso':
+            sys_batch_size = ["auto" for aa in range(len(jdata['sys_configs']))]
+        else: 
+            sys_batch_size = ["auto" for aa in range(3000)]


Where does the "3000" come from?

amcadmus · 2022-03-20T04:34:31Z

dpgen/generator/run.py

+    else:
+        try:
+            maxiter = max(model_devi_jobs[-1].get('times'))
+        except Exception as e:


It is better to specify which exception you want to catch.

amcadmus · 2022-03-20T04:38:10Z

dpgen/generator/run.py

-                    all_sys.append(_sys)
+                    try:
+                        all_sys.append(_sys)
+                    except:


It is better to specify which exception you want to catch.

amcadmus · 2022-03-20T04:41:11Z

dpgen/generator/run.py

+                    try:
+                        all_sys.append(_sys)
+                    except:
+                        dlog.info('%s has different formula, so pass in line 3518'%oo)


It is not a good idea to hard coding the line number.

Please check the solutions : https://stackoverflow.com/questions/3056048/filename-and-line-number-of-python-script

AnguseZhang

@wangzyphysics @wanghan-iapcm Please see my comments. Actually I have a lot of concerns. We may have a talk if necessary.

AnguseZhang · 2022-03-22T08:14:52Z

README.md

@@ -378,7 +378,7 @@ In each iteration, there are three stages of work, namely, `00.train  01.model_d

 + 00.train: DP-GEN will train several (default 4) models based on initial and generated data. The only difference between these models is the random seed for neural network initialization.

-+ 01.model_devi : represent for model-deviation. DP-GEN will use models obtained from 00.train to run Molecular Dynamics(default LAMMPS). Larger deviation for structure properties (default is force of atoms) means less accuracy of the models. Using this criterion, a few fructures will be selected and put into next stage `02.fp` for more accurate calculation based on First Principles.
+ 01.model_devi : represent for model-deviation. Model-deviation engine in `01.model_devi` can be chosen between Molecular Dynamics(default LAMMPS) or Structures Prediction(CALYPSO). DP-GEN will use models obtained from 00.train to run Molecular Dynamics or to run structure optimization with ASE in CALYPSO. Larger deviation for structure properties (default is force of atoms) means less accuracy of the models. Using this criterion, a few fructures will be selected and put into next stage `02.fp` for more accurate calculation based on First Principles.


Could you please change "default LAMMPS" to "LAMMPS and GROMACS" ?

AnguseZhang · 2022-03-22T08:23:01Z

dpgen/generator/lib/calypso.py

+    ret+="    f.write('in kB')                                                                    \n"
+    ret+="    f.write('%15.6f' % stress[0])                                                       \n"
+    ret+="    f.write('%15.6f' % stress[1])                                                       \n"
+    ret+="    f.write('%15.6f' % stress[2])                                                       \n"


I strongly suggest put these codes in a complete script, like lib/xxx.py, instead of making them strings like you did. On the one hand, the code is easier to read and maintain. On the other hand, users can directly run the script if they have needs.
If you need to execute the script, you can copy the script to you current path.
You can refer to

dpgen/dpgen/generator/run.py

Line 1911 in 93b833a

shutil.copyfile(cvasp_file, 'cvasp.py')

AnguseZhang · 2022-03-22T08:34:17Z

dpgen/generator/run.py

+from dpgen.generator.lib.calypso import make_run_opt_script,make_check_outcar_script
+from dpgen.generator.lib.calypso import _make_model_devi_native_calypso,write_model_devi_out,_make_model_devi_buffet
+from dpgen.generator.lib.model_devi_calypso import gen_structures,analysis
+from dpgen.generator.lib.write_modd import write_modd


As @amcadmus suggests, such functions should be named write_model_devi_calypso, etc.

AnguseZhang · 2022-03-22T08:38:58Z

dpgen/generator/lib/model_devi_calypso.py

+    calypso_model_devi_path = os.path.join(work_path,calypso_model_devi_name)
+    #
+
+    calypso_path = jdata.get('calypso_path')


The parameters about machines and environment variables should be added to machine.json, not param.json

AnguseZhang · 2022-03-22T08:42:10Z

dpgen/generator/lib/model_devi_calypso.py

+#    write_model_devi_out(Devis,'Model_Devi.out')
+#    os.chdir(cwd)
+#
+#    os.chdir(work_path)


Please delete all commented codes.

AnguseZhang · 2022-03-22T08:56:49Z

dpgen/generator/run.py

@@ -1327,6 +1385,68 @@ def run_model_devi (iter_index,
            errlog = 'model_devi.log')
        submission.run_submission()

+def run_model_devi(iter_index,jdata,mdata):


These codes will fundamentally change DP-GEN's protocol by adding a while loop in run_model_devi. Is it possible to put all calypso commands to one single script, instead of DP-GEN's main process?

- rename model_devi_calypso.py run_calypso.py - rename calypso.py make_calypso.py - add new file calypso_check_outcar.py calypso_run_opt.py - add new file calypso_run_model_devi.py - delete write_modd.py - modify README.md and run.py

…fy test_calypso.py

wanghan-iapcm · 2022-03-24T05:13:39Z

dpgen/generator/run.py

@@ -2537,8 +2708,13 @@ def post_fp_vasp (iter_index,
                if all_sys is None:
                    all_sys = _sys
                else:
+                    #try:


You may want to remove the commented code.

wanghan-iapcm · 2022-03-24T05:14:26Z

dpgen/generator/run.py

                # save ele_temp, if any
+                # need to check 


what is needed to check?

wanghan-iapcm

Only a few comments are left. I agree the merging of this PR.

…ittest

… fix bug in runopt script

amcadmus · 2022-03-28T02:01:07Z

Please fix the failure in the UTs.

amcadmus and others added 25 commits August 27, 2021 21:56

Merge pull request deepmodeling#523 from deepmodeling/devel

e9d5952

Merge devel into master

fix typo in github actions release to conda

fb30edf

Merge pull request deepmodeling#524 from felix5572/master

9dbac8d

fix typo in github actions release to conda

Get from_poscar_path only when from_poscar is true (deepmodeling#537)

02037ad

Change-Id: I17774bee345634e4e72bd783e8112eefaaf9f0d3 Co-authored-by: Zhengju Sha <[email protected]>

Merge branch 'master' into devel

eb6449c

Merge pull request deepmodeling#613 from amcadmus/master

93b833a

Merge recent devel into master

feature: merge run.py

12971b9

fix:fix syntax bug

3b26eb9

fix summary ratio bug

729d8bd

fix: fix bug of var(calypso_run_opt_path) not defined

e08b479

Merge branch 'deepmodeling:master' into calypso

3d227a9

reorganize code v1

7d30fc1

add: add unittest

7c68fe9

Merge branch 'calypso' of github.com:wangzyphysics/dpgen into calypso

86b9cf2

Merge 'calypso' from origin into 'calypso'

add: add some explainations of dp+calypso in README.md

420df0d

change a little bit in readme.md

3277c26

change position of function

09be223

add deepmd-kit>=2.0.1 install_require in setup.py

224b62d

test failed cause no tensorflow module so add tensorflow in setup.py

8af2b2a

calc model devi in command line

239765a

fix some small bugs of modd and PBS script of cpu

0bf1473

fix unittest

7b0db3f

delete parse_input.dat

15d130a

fix unittest

d1e73c9

first reorganization

361546e

secord reorganization

a8ca7f8

fix bugs

2ac1efe

fix: fix bugs in modd_calypso.py write_modd.py and run.py

f1ae450

amcadmus requested a review from AnguseZhang March 20, 2022 03:42

amcadmus requested changes Mar 20, 2022

View reviewed changes

wangzyphysics added 5 commits March 22, 2022 15:21

change functions name and fix bugs

a6048fb

remove modd_calypso.py

d651e36

remove: remove modd_calypso.py

1c1b670

remove some useless outputs

68cd1c9

restore dispatcher/PBS.py

ff23f23

wangzyphysics requested a review from amcadmus March 22, 2022 08:13

AnguseZhang requested changes Mar 22, 2022

View reviewed changes

wangzyphysics added 3 commits March 23, 2022 16:46

refactor: make some changes in calypso related code

3796cd4

- rename model_devi_calypso.py run_calypso.py - rename calypso.py make_calypso.py - add new file calypso_check_outcar.py calypso_run_opt.py - add new file calypso_run_model_devi.py - delete write_modd.py - modify README.md and run.py

refactor: move calypso main code(run.py) into run_calypso.py and modi…

496fa07

…fy test_calypso.py

docs: modify README.md related to CALYPSO

36dd23a

wanghan-iapcm reviewed Mar 24, 2022

View reviewed changes

dpgen/generator/run.py Outdated

# save ele_temp, if any

# need to check

Copy link

Contributor

wanghan-iapcm Mar 24, 2022

Choose a reason for hiding this comment

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

what is needed to check?

wanghan-iapcm approved these changes Mar 24, 2022

View reviewed changes

wangzyphysics added 4 commits March 24, 2022 17:05

delete some useless comments in run.py

0e9669e

add write_model_devi_out function into make_calypso.py for passing un…

961f63f

…ittest

fix a spelling mistake in calypso_run_opt.py (famx -> fmax)

df2bf1c

change runopt script's name from run_opt.py to calypso_run_opt.py and…

43ef866

… fix bug in runopt script

amcadmus requested a review from AnguseZhang March 28, 2022 02:01

fix unittest

33271e9

wanghan-iapcm merged commit 37df129 into deepmodeling:devel Apr 20, 2022

njzjz mentioned this pull request May 9, 2024

[Feature Request] problem with “maybe the model devi is nan” #1543

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calypso #663

Calypso #663

wangzyphysics commented Mar 10, 2022

codecov-commenter commented Mar 13, 2022 •

edited

Loading

AnguseZhang commented Mar 14, 2022

wanghan-iapcm commented Mar 15, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

amcadmus Mar 20, 2022

AnguseZhang left a comment

AnguseZhang Mar 22, 2022

AnguseZhang Mar 22, 2022

AnguseZhang Mar 22, 2022

AnguseZhang Mar 22, 2022

AnguseZhang Mar 22, 2022

AnguseZhang Mar 22, 2022

wanghan-iapcm Mar 24, 2022

wanghan-iapcm Mar 24, 2022

wanghan-iapcm left a comment

amcadmus commented Mar 28, 2022

Calypso #663

Calypso #663

Conversation

wangzyphysics commented Mar 10, 2022

codecov-commenter commented Mar 13, 2022 • edited Loading

Codecov Report

AnguseZhang commented Mar 14, 2022

wanghan-iapcm commented Mar 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnguseZhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wanghan-iapcm left a comment

Choose a reason for hiding this comment

amcadmus commented Mar 28, 2022

codecov-commenter commented Mar 13, 2022 •

edited

Loading