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

Refactor and unify the random numbers. #42

Merged
merged 9 commits into from
Jul 2, 2024
Merged

Refactor and unify the random numbers. #42

merged 9 commits into from
Jul 2, 2024

Conversation

cianciosa
Copy link
Collaborator

This unifies and cleans up the random number generation to ensure that random numbers are seeded once at the beginning.

@cianciosa cianciosa marked this pull request as draft May 31, 2024 16:22
@quantumsteve
Copy link
Collaborator

@idigs @cianciosa Do you see an error message from job_two?

@cianciosa
Copy link
Collaborator Author

@idigs @cianciosa Do you see an error message from job_two?

I'm guessing the test failure is a result of differences in seeding. Do you know if this test uses a constant seed?

@mbeidler3
Copy link
Collaborator

The initial case I provided may use a different seed than the present case.

@quantumsteve
Copy link
Collaborator

@idigs @cianciosa Do you see an error message from job_two?

I'm guessing the test failure is a result of differences in seeding. Do you know if this test uses a constant seed?

I think it's a Chrome or GitHub issue.

@quantumsteve
Copy link
Collaborator

image

@cianciosa
Copy link
Collaborator Author

I get

Screenshot 2024-05-31 at 2 23 24 PM

Copy link
Collaborator

@mbeidler3 mbeidler3 left a comment

Choose a reason for hiding this comment

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

This is looking pretty good on my computer. When I run it multiple times, I keep getting the same exact results with a good distributions of REs. The only issue is that I am getting different answers compared to what Mark sent me by email on 6/5/24. While the spatial distribution is different.

@cianciosa cianciosa marked this pull request as ready for review July 1, 2024 17:39
@mbeidler3
Copy link
Collaborator

test this please

@mbeidler3
Copy link
Collaborator

@idigs it looks like the SPACK cache is getting built and created again. Can we set it up a workflow to have this to happen automatically on a weekend evening?

@mbeidler3
Copy link
Collaborator

test this please

@mbeidler3
Copy link
Collaborator

@idigs, how long does it usually take for an ExCL runner to pick up the Onyx workflow?

Copy link
Collaborator

@mbeidler3 mbeidler3 left a comment

Choose a reason for hiding this comment

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

Didn't do comprehensive testing due to size, but works for mars regression CTest.

@mbeidler3 mbeidler3 merged commit 30baea8 into main Jul 2, 2024
2 checks passed
@mbeidler3 mbeidler3 deleted the modern_random branch July 2, 2024 18:11
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