Skip to content

add ML labeling option for vasp ml-aimd OUTCAR#282

Merged
wanghan-iapcm merged 12 commits into
deepmodeling:develfrom
PKUfjh:devel
May 14, 2022
Merged

add ML labeling option for vasp ml-aimd OUTCAR#282
wanghan-iapcm merged 12 commits into
deepmodeling:develfrom
PKUfjh:devel

Conversation

@PKUfjh

@PKUfjh PKUfjh commented May 12, 2022

Copy link
Copy Markdown
Contributor

I fixed bugs in the previous PR #278.
I also added test cases for vasp machine learning force fields MD OUTCAR

This PR intends to add labeling option for VASP machine learning force field AIMD, the default setting is the same with the original repo, which selects the first principle calculation among the MD frames as energy, force and viral labels. But one can also add option in the LabeledSystem like

LabeledSystem(filename,fmt='vasp/outcar',ml=True)

to use the machine learned energy, force and viral labels.
This option obviously is not as accurate as the default fp labels, but the motivation for adding this option (for me) is mainly to sample the structures rather than use the labeled system as training set. And you only need to change a little bit to realize the function. Also, since the default setting is the same as the original one, there is little worry to use this option in a wrong way.

Comment thread dpdata/plugins/vasp.py Outdated
Comment on lines +59 to +63
ml = False
if kwargs is not None:
for key, value in kwargs.items():
if key=="ml":
ml = value

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These lines can be written as one line

ml = kwargs.get("ml", False)

or it can be directly written in the def line

@PKUfjh PKUfjh May 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed the code into one line

Comment thread dpdata/plugins/vasp.py Outdated
data['forces'], \
tmp_virial, \
= dpdata.vasp.outcar.get_frames(file_name, begin=begin, step=step)
= dpdata.vasp.outcar.get_frames(file_name, begin=begin, step=step,ml=ml)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line does not follow PEP-8.

@PKUfjh PKUfjh May 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added whitespace after the comma

Comment thread dpdata/plugins/vasp.py Outdated
Comment thread dpdata/vasp/outcar.py Outdated

# we assume that the force is printed ...
def get_frames (fname, begin = 0, step = 1) :
def get_frames (fname, begin = 0, step = 1,ml = False) :

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line does not follow PEP-8.

Comment thread dpdata/vasp/outcar.py Outdated
Comment thread dpdata/vasp/outcar.py Outdated
Comment on lines +174 to +179
# all_coords.append(coord)
# all_cells.append(cell)
# all_energies.append(energy)
# all_forces.append(force)
# if virial is not None :
# all_virials.append(virial)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The unrelated comment block should be removed.

Comment thread dpdata/vasp/outcar.py Outdated
Comment thread dpdata/vasp/outcar.py Outdated
energy = None
force = []
virial = None
for idx,ii in enumerate(lines) :

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line does not obey PEP-8.

Comment thread dpdata/vasp/outcar.py Outdated
coord, cell, energy, force, virial = analyze_block_ml(blk, ntot)
if len(coord) == 0:
break
break

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The number of the indent is wrong.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed the indent into four spaces

Comment thread dpdata/vasp/outcar.py Outdated
coord, cell, energy, force, virial = analyze_block_ml(blk, ntot)
if len(coord) == 0:
break
break

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The number of the indent is wrong.

@wanghan-iapcm wanghan-iapcm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the difference between the analyze_block and analyze_block_ml. It seems that the only differences are the keyword and the number of shifted lines. If so it is recommended to merge the two functions.

@codecov-commenter

codecov-commenter commented May 13, 2022

Copy link
Copy Markdown

Codecov Report

Merging #282 (49c2673) into devel (6a147ea) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            devel     #282      +/-   ##
==========================================
+ Coverage   81.57%   81.61%   +0.03%     
==========================================
  Files          62       62              
  Lines        5173     5183      +10     
==========================================
+ Hits         4220     4230      +10     
  Misses        953      953              
Impacted Files Coverage Δ
dpdata/plugins/vasp.py 100.00% <100.00%> (ø)
dpdata/vasp/outcar.py 96.66% <100.00%> (+0.27%) ⬆️

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 6a147ea...49c2673. Read the comment docs.

@PKUfjh

PKUfjh commented May 13, 2022

Copy link
Copy Markdown
Contributor Author

#282 (review)

I merge functions analyze_block and analyze_block_ml into one, and also merge get_outcar_block and get_outcar_block_ml into one.

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.

4 participants