Skip to content

fix(abacus): do not collect the unconverged structure for AIMD#368

Merged
wanghan-iapcm merged 9 commits intodeepmodeling:develfrom
pxlxingliang:md
Oct 29, 2022
Merged

fix(abacus): do not collect the unconverged structure for AIMD#368
wanghan-iapcm merged 9 commits intodeepmodeling:develfrom
pxlxingliang:md

Conversation

@pxlxingliang
Copy link
Contributor

No description provided.

@pxlxingliang pxlxingliang changed the title Md fix(abacus): do not collect the unconverged structure for AIMD Oct 26, 2022
nenergy+=1
elif "!! convergence has not been achieved" in line:
if nenergy%dump_freq == 0:
energy.append(0)
Copy link
Member

Choose a reason for hiding this comment

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

If 0 is used to indicate systems that do not coverage, how can we collect 0-energy systems?

Copy link
Contributor Author

@pxlxingliang pxlxingliang Oct 27, 2022

Choose a reason for hiding this comment

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

0 is really not a good choice. May "None" is better. I will change it.

Copy link
Member

Choose a reason for hiding this comment

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

Can ABACUS calculate a hydrogen ion?

Copy link
Member

Choose a reason for hiding this comment

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

Note that None will make the NumPy array become an object array:

>>> import numpy as np
>>> np.array([1., None]).dtype
dtype('O')

Maybe you can use np.nan or np.inf instead.

>>> np.array([1., np.nan, np.inf]).dtype
float64
>>> x = np.array([1., np.nan, np.inf])
>>> np.isnan(x)
array([False,  True, False])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can ABACUS calculate a hydrogen ion?

Yes. I see. The previous statement is indeed incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that None will make the NumPy array become an object array:

>>> import numpy as np
>>> np.array([1., None]).dtype
dtype('O')

Maybe you can use np.nan or np.inf instead.

>>> np.array([1., np.nan, np.inf]).dtype
float64
>>> x = np.array([1., np.nan, np.inf])
>>> np.isnan(x)
array([False,  True, False])

Thanks for your suggestion, I don't know about the object array before.
I will change to "np.nan"

@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2022

Codecov Report

Base: 82.33% // Head: 82.21% // Decreases project coverage by -0.11% ⚠️

Coverage data is based on head (5bfd1d1) compared to base (e8a2fbe).
Patch coverage: 41.17% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel     #368      +/-   ##
==========================================
- Coverage   82.33%   82.21%   -0.12%     
==========================================
  Files          67       67              
  Lines        5892     5909      +17     
==========================================
+ Hits         4851     4858       +7     
- Misses       1041     1051      +10     
Impacted Files Coverage Δ
dpdata/abacus/md.py 90.75% <41.17%> (-8.27%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@njzjz
Copy link
Member

njzjz commented Oct 27, 2022

Can you add an extra test?

@pxlxingliang
Copy link
Contributor Author

Can you add an extra test?

Ok, I will add one.

@wanghan-iapcm wanghan-iapcm merged commit bc6508b into deepmodeling:devel Oct 29, 2022
@pxlxingliang pxlxingliang deleted the md branch January 19, 2023 06:42
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