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

edmf random seed not too random? #614

Closed
mathomp4 opened this issue Jul 28, 2022 · 4 comments · Fixed by #783
Closed

edmf random seed not too random? #614

mathomp4 opened this issue Jul 28, 2022 · 4 comments · Fixed by #783

Comments

@mathomp4
Copy link
Member

@narnold1 A question. While talking with NVIDIA about their GPU efforts with edmf we looked at this code:

! get Poisson P(dz/L0)
seedmf(1) = 1000000 * ( 100*thl(kte) - INT(100*thl(kte)))
seedmf(2) = 1000000 * ( 100*thl(kte-1) - INT(100*thl(kte-1)))
THE_SEED(1)=seedmf(1) + seedmf(2)
THE_SEED(2)=seedmf(1) + seedmf(2)
THE_SEED(1)=THE_SEED(1)*seedmf(1)/( seedmf(2) + 10)
THE_SEED(2)=THE_SEED(2)*seedmf(1)/( seedmf(2) + 10)
if(THE_SEED(1) == 0) THE_SEED(1) = 5
if(THE_SEED(2) == 0) THE_SEED(2) = -5

and then the_seed is passed to Poisson:

The thing is...I'm pretty sure the_seed(1) equals the_seed(2) unless they are 0. We were thinking maybe it should be:

 THE_SEED(1)=seedmf(1) + seedmf(2) 
 THE_SEED(2)=seedmf(1) - seedmf(2) 

at the beginning?

Just wondering.

Cc: @tclune as he probably knows more about algorithmically making random numbers than I do

@tclune
Copy link
Collaborator

tclune commented Aug 1, 2022

It all looks very ad hoc to me. Some attempt to pre-randomize a seed?

@narnold1
Copy link
Contributor

narnold1 commented Aug 1, 2022

This section of code was copied from ras.F90. I'm not sure what motivated lines 351-356. Is THE_SEED=0 disallowed? I think the current code works, in the sense of producing randomness and satisfying regression tests, but I will double check that.

@mathomp4
Copy link
Member Author

mathomp4 commented Aug 2, 2022

This section of code was copied from ras.F90. I'm not sure what motivated lines 351-356. Is THE_SEED=0 disallowed? I think the current code works, in the sense of producing randomness and satisfying regression tests, but I will double check that.

@narnold1 There are compilers where setting a seed that's not too random causes issues (see https://forums.developer.nvidia.com/t/random-number-seed-oddities/133398 by some "Matt Thompson"), so I always discourage that once I realized that. I mean, setting the seed based on input temperatures isn't bad, it's just like the seed construction was a bit oddly coded. 😄

@narnold1 narnold1 linked a pull request Aug 1, 2023 that will close this issue
@narnold1
Copy link
Contributor

narnold1 commented Aug 1, 2023

This issue is addressed by PR #783

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 a pull request may close this issue.

3 participants