Skip to content

Bug fix of memory overflow when calculating model deviation#1154

Merged
amcadmus merged 33 commits intodeepmodeling:develfrom
Ericwang6:devel
Sep 26, 2021
Merged

Bug fix of memory overflow when calculating model deviation#1154
amcadmus merged 33 commits intodeepmodeling:develfrom
Ericwang6:devel

Conversation

@Ericwang6
Copy link
Member

Calculate model deviation frame by frame to avoid memory overflow problems

Ericwang6 and others added 30 commits June 2, 2021 19:29
@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2021

Codecov Report

Merging #1154 (48f5bdf) into devel (077df3d) will increase coverage by 0.39%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel    #1154      +/-   ##
==========================================
+ Coverage   75.66%   76.06%   +0.39%     
==========================================
  Files          89       91       +2     
  Lines        7048     7241     +193     
==========================================
+ Hits         5333     5508     +175     
- Misses       1715     1733      +18     
Impacted Files Coverage Δ
deepmd/infer/model_devi.py 52.43% <0.00%> (-0.65%) ⬇️
deepmd/train/trainer.py 73.89% <0.00%> (-0.42%) ⬇️
deepmd/utils/errors.py 100.00% <0.00%> (ø)
deepmd/utils/batch_size.py 96.00% <0.00%> (ø)
deepmd/utils/path.py 91.45% <0.00%> (ø)
deepmd/utils/argcheck.py 90.00% <0.00%> (+0.03%) ⬆️
deepmd/utils/data.py 91.31% <0.00%> (+0.05%) ⬆️
deepmd/entrypoints/train.py 87.57% <0.00%> (+0.14%) ⬆️
deepmd/common.py 84.28% <0.00%> (+0.22%) ⬆️
deepmd/entrypoints/test.py 11.90% <0.00%> (+0.36%) ⬆️
... and 2 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 077df3d...48f5bdf. Read the comment docs.

devis.append(devi)
coords = data["coord"]
boxs = data["box"]
atype = data["type"][0]
Copy link
Member

@njzjz njzjz Sep 15, 2021

Choose a reason for hiding this comment

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

Is there a reason to only use the first (?) atom type?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think atom types for all frames in a dataset is the same, aren't they?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we can publicly ensure it if the returned array is the size of nframes*natoms instead of natoms

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @njzjz , in this context we do not assume all frames of data["type"] has the same value.

devis.append(devi)
coords = data["coord"]
boxs = data["box"]
atype = data["type"][0]
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @njzjz , in this context we do not assume all frames of data["type"] has the same value.

@amcadmus
Copy link
Member

Next time you PR, please checkout a new branch.
your devel has too many commits that do not exist on the current devel branch.

@amcadmus
Copy link
Member

fixed by #1165

@amcadmus amcadmus closed this Sep 24, 2021
@njzjz
Copy link
Member

njzjz commented Sep 24, 2021

fixed by #1165

I don't think so... dp test and dp model-devi are two different entry points.

@amcadmus amcadmus reopened this Sep 24, 2021
@Ericwang6 Ericwang6 requested review from amcadmus and njzjz September 24, 2021 16:25
@amcadmus
Copy link
Member

@njzjz It should be straightforward of using AutoBatchSize here, what do you think?

@njzjz
Copy link
Member

njzjz commented Sep 25, 2021

@njzjz It should be straightforward of using AutoBatchSize here, what do you think?

Agree, and I've not decided whether we should use it on DeepPot -- the Python interface provided to users.

@amcadmus amcadmus merged commit d8acbb8 into deepmodeling:devel Sep 26, 2021
@amcadmus
Copy link
Member

@njzjz It should be straightforward of using AutoBatchSize here, what do you think?

Agree, and I've not decided whether we should use it on DeepPot -- the Python interface provided to users.

Good idea!

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