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

Update gromacs model devi engine #493

Merged
merged 15 commits into from Aug 14, 2021
Merged

Conversation

Ericwang6
Copy link
Member

  1. Update gromacs model devi engine, supporting multi lambdas, multi temperatures of simulation.
  2. Update gaussian fp engine, supporting different charges for different systems.
  3. Bug fix of UnboundLocalError when parsing gaussian log

@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2021

Codecov Report

Merging #493 (1e7e516) into devel (ddf39f1) will increase coverage by 0.08%.
The diff coverage is 69.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #493      +/-   ##
==========================================
+ Coverage   32.97%   33.06%   +0.08%     
==========================================
  Files          86       86              
  Lines       14560    14717     +157     
==========================================
+ Hits         4801     4866      +65     
- Misses       9759     9851      +92     
Impacted Files Coverage Δ
dpgen/generator/lib/gaussian.py 31.57% <66.66%> (+0.19%) ⬆️
dpgen/generator/run.py 65.88% <70.00%> (-2.15%) ⬇️
dpgen/generator/lib/lammps.py 72.59% <0.00%> (+4.32%) ⬆️

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 ddf39f1...1e7e516. Read the comment docs.

if idx == 0:
if jdata.get('use_clusters', False):
all_sys = dpdata.MultiSystems(sys, type_map = jdata['type_map'])
# TODO : UnboundLocalError sometimes occurs and I cannot figure it out.
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 UnboundLocalError is an expected error that should be caught. If it happened, it is a bug that needs to be fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. I met this problem this week but I don't have time to fix it :(
I will remove this part and try to fix it afterwards.

system = dpdata.System(dump, fmt = fmt, type_map = type_map)
system.to_deepmd_raw(deepmd_raw)
if charge is not None:
with open(os.path.join(deepmd_raw, "charge"), 'w+') as f:
Copy link
Member

Choose a reason for hiding this comment

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

Why using w+ instead of w?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's my own habits. It seems there are not so many differences between them ?

Copy link
Member

Choose a reason for hiding this comment

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

w+ will create both reading and writing buffers instead of only writing buffer.

Comment on lines 1104 to 1105
lambdas = cur_job.get("lambdas", [])
temps = cur_job.get("temps", [])
Copy link
Member

Choose a reason for hiding this comment

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

You can put the default value here... like

Suggested change
lambdas = cur_job.get("lambdas", [])
temps = cur_job.get("temps", [])
lambdas = cur_job.get("lambdas", [1.])
temps = cur_job.get("temps", [298.])

and remove lines 1106-1107, 1112-1113

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it.

Copy link
Collaborator

@AnguseZhang AnguseZhang left a comment

Choose a reason for hiding this comment

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

Examples including sys_charges should be updated

dpgen/generator/run.py Outdated Show resolved Hide resolved
dpgen/generator/run.py Outdated Show resolved Hide resolved
dpgen/generator/run.py Show resolved Hide resolved
@AnguseZhang AnguseZhang merged commit 6af59ba into deepmodeling:devel Aug 14, 2021
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.

None yet

4 participants