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

added uncertainty estimation on dump interval for jaxmd #312

Merged
merged 22 commits into from
Aug 9, 2024
Merged

Conversation

M-R-Schaefer
Copy link
Contributor

A full model is evaluated when a configuration is dumped.
Before we would evaluate an energy model at every time step but only collect the energy aver N steps, which was a bit redundant. Now we can save all model outputs.

closes #298

@M-R-Schaefer M-R-Schaefer marked this pull request as ready for review August 5, 2024 09:05
@M-R-Schaefer
Copy link
Contributor Author

also closes #309

@M-R-Schaefer
Copy link
Contributor Author

pre-commit.ci autofix

def body_fn(i, state):
state, neighbor = state
state, outer_step, neighbor = state # neighbors
Copy link
Contributor

Choose a reason for hiding this comment

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

don't understand the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment is superfluous and can be removed.

if isinstance(state, simulate.NPTNoseHooverState):
box = state.box
apply_fn_kwargs = {}
else:
box = system.box
apply_fn_kwargs = {"box": box}
apply_fn_kwargs = {"box": box} # this might cause a bug
Copy link
Contributor

Choose a reason for hiding this comment

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

should we discuss this bevor merging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment was just a reminder for myself to look into this. it's all working as intended. I'll remove the comment

Copy link
Contributor

@Tetracarbonylnickel Tetracarbonylnickel left a comment

Choose a reason for hiding this comment

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

After removing the comments it looks fine from me.

@M-R-Schaefer M-R-Schaefer merged commit 5b9df07 into main Aug 9, 2024
1 of 2 checks passed
@M-R-Schaefer M-R-Schaefer deleted the jamd_uq branch August 9, 2024 08:25
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.

Add uncertainties to jaxmd
2 participants