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

ran_unif Mersenne twister test: Resolution to Issue #499 #549

Merged
merged 12 commits into from
Oct 5, 2023

Conversation

c-merchant
Copy link
Collaborator

@c-merchant c-merchant commented Sep 26, 2023

Description:

Created ran_twist function that preserves the integer from ran_unif Mersenne twister PRNG.
Created test_ran_unif that runs ran_twist with a known seed, compare the integers produced against a reference solution for the Mersenne Twister.

Fixes issue

fixes #499

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Documentation changes needed?

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.

Tests

Please describe any tests you ran to verify your changes.
I ran ./quickbuild.sh and ./runall.sh in ~/DART/developer_tests/random_seq/work

Checklist for merging

  • Updated changelog entry
  • Documentation updated
  • Update conf.py

Checklist for release

  • Merge into main
  • Create release from the main branch with appropriate tag
  • Delete feature-branch

Testing Datasets

  • Dataset needed for testing available upon request
  • Dataset download instructions included
  • No dataset needed

Copy link
Member

@hkershaw-brown hkershaw-brown left a comment

Choose a reason for hiding this comment

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

Hi Charlotte,

The public ran_twist routine to return the integer value has the correct code. The comments need updating to match what the routine returns (integer).

The new program test_ran_unif is not being compiled. You can check this with

./quickbuild.sh test_rand_unif
...
ERROR: unknown program test_rand_unif

the CCE 15 compiler on Derecho should fail your test ./test_rand_unif

The test_ran_unif is calling the new routine, and pass/failing. It has a bunch of legacy code from test_random which we don't really want to commit. I'll add some notes on what you can pull out shortly.

Cheers,
Helen

@hkershaw-brown hkershaw-brown changed the title Resolution to Issue #499 ran_unif Mersenne twister test: Resolution to Issue #499 Sep 28, 2023
@hkershaw-brown
Copy link
Member

@c-merchant looks good Charlotte!
Produces a fail for CCE 15 on Derecho, pass for other compilers.
There's some setup with ntests, and "save values for plotting" that we don't need for this test (the test passes or it fails). I'm going to go ahead and strip those out and we can bundle this with the next release.

Cheers,
Helen

@hkershaw-brown hkershaw-brown added the release! bundle with next release label Sep 29, 2023
Copy link
Member

@hkershaw-brown hkershaw-brown left a comment

Choose a reason for hiding this comment

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

Approved!

@hkershaw-brown hkershaw-brown merged commit 6c6ac53 into NCAR:main Oct 5, 2023
2 checks passed
@hkershaw-brown
Copy link
Member

@hkershaw-brown you missed these two commits off this pull request
main...499

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release! bundle with next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Pass/fail test for ran_unif
2 participants