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

gamma line calculation for bcc and fcc #801

Merged
merged 66 commits into from
Aug 3, 2022

Conversation

kevinwenminion
Copy link
Collaborator

  1. add gamma line calculation for bcc and fcc.

  2. should be no problem for bcc {110}, {112}, and {123} planes along <111>/2 direction; fcc {111} plane along <1-10>/2 and <11-2>/2 directions.

@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2022

Codecov Report

Merging #801 (5646f1d) into devel (b784351) will increase coverage by 1.61%.
The diff coverage is 53.61%.

@@            Coverage Diff             @@
##            devel     #801      +/-   ##
==========================================
+ Coverage   33.91%   35.53%   +1.61%     
==========================================
  Files          93       97       +4     
  Lines       16562    17077     +515     
==========================================
+ Hits         5617     6068     +451     
- Misses      10945    11009      +64     
Impacted Files Coverage Δ
dpgen/data/reaction.py 0.00% <0.00%> (ø)
dpgen/dispatcher/DispatcherList.py 0.00% <0.00%> (ø)
dpgen/generator/arginfo.py 45.37% <ø> (+45.37%) ⬆️
dpgen/main.py 0.00% <0.00%> (ø)
dpgen/simplify/simplify.py 0.00% <0.00%> (ø)
dpgen/dispatcher/Dispatcher.py 31.64% <22.22%> (-0.35%) ⬇️
dpgen/arginfo.py 33.33% <33.33%> (ø)
dpgen/auto_test/common_prop.py 38.15% <40.00%> (-0.10%) ⬇️
dpgen/auto_test/Gamma.py 61.13% <61.13%> (ø)
dpgen/data/arginfo.py 87.50% <62.50%> (ø)
... and 13 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 b784351...5646f1d. Read the comment docs.

Copy link
Collaborator

@Vibsteamer Vibsteamer left a comment

Choose a reason for hiding this comment

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

Dear contributors,
There are five points maybe needed for further consideration:

1.dpgen/auto_test/Gamma.py, the "min" in the key of parameters "min_super_cell_size" seems not intuitive, which key just refers to the replication times in three dimensions of the super-cell with respect to the primitive cell. (had communicated with the contributor)
2.dpgen/auto_test/Gamma.py, the function __gen_slab_pmg is defined but is not called elsewhere, seem to be a dangling function for the time being. If it is prepared for further developments in future pr, adding some comments maybe more friendly to other developers/code_readers.
3.POTCAR files should bot be uploaded for copyrights limitation
4.for giving the example and unitest, Maybe OUTCAR files could be uploaded instead, and corresponding test of post process could be added into the unitest.
5.dpgen/auto_test/Lammps.py, the default convergence criteria for lammps tasks are changed into force-dependent-only, which may influence all other properties in auto-test. Please add a comment explaining the consideration of this revise.

@wanghan-iapcm
Copy link
Contributor

Please checkout a new branch and make a new PR for the cohesive energy line. @kevinwenminion

@kevinwenminion
Copy link
Collaborator Author

Dear reviewers,

Problems 1-4 have been solved and the unitest for gamma-line post for lmp calculation has been added.

For the last question, when relaxing the supercell with many atoms, force convergence is much more robust than energy convergence. From our experience, the force convergence of 1e-10 for DP should be OK in most situations.

Thanks a lot for your careful review and time.

@AnguseZhang
Copy link
Collaborator

Thanks for the contributions! But there're so many output files, are they necessary?

@wanghan-iapcm wanghan-iapcm merged commit a78f8a5 into deepmodeling:devel Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants