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

Testing/forces update #1076

Merged
merged 28 commits into from
Sep 13, 2023
Merged

Testing/forces update #1076

merged 28 commits into from
Sep 13, 2023

Conversation

shuds13
Copy link
Member

@shuds13 shuds13 commented Sep 6, 2023

Update forces tests.

For relevant forces tests

  • Use persistent generator as default
  • Use new OO interface

Other capabilities

  • Add generator that sets num_gpus based on x value in range(e.g. number of particles)
  • Update regression test test_GPU_variable_resources.py to use new gen (as well as old).

Tests

  • Updating forces_simple and forces_gpu and prob. forces_multi_task/
  • Remove forces_gpu_persis_gen - as now that's default
  • Add a var resources test (separate now - and using new num_gpus generator)

Update tutorials

  • Forces simple
  • Forces GPU

@shuds13 shuds13 added the Testing label Sep 6, 2023
@shuds13 shuds13 self-assigned this Sep 6, 2023
@shuds13 shuds13 marked this pull request as ready for review September 12, 2023 23:33
@shuds13 shuds13 requested review from jlnav and jmlarson1 September 12, 2023 23:33
# Seed random streams for each worker, particularly for gen_f
persis_info = add_unique_random_streams({}, nworkers + 1)
# Seed random streams for each worker, particularly for gen_f
ensemble.add_random_streams()
Copy link
Member

Choose a reason for hiding this comment

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

You could use ensemble.persis_info = add_unique_random_streams({}, ensemble.nworkers + 1) if you think its more clear

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I'm realising it means an extra import (from tools) which is a pain.

Copy link
Member

@jlnav jlnav Sep 13, 2023

Choose a reason for hiding this comment

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

Yeah. That's one of the reasons I made Ensemble.add_random_streams() originally, I think I forgot to mention. Avoid an import, keep it simple for 75% of our tests

Copy link
Member

@jlnav jlnav left a comment

Choose a reason for hiding this comment

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

Looks good!

@shuds13 shuds13 merged commit e6510e8 into develop Sep 13, 2023
3 of 13 checks passed
@jmlarson1 jmlarson1 deleted the testing/forces_update branch September 21, 2023 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants