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

restore RNG state after generating random numbers #353

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Conversation

m-bossart
Copy link
Collaborator

For generating datasets from PSID simulations it is useful to rely on a seed and random number generator to randomly select a perturbation. Currently, it is not possible to evaluate the same set of randomly selected perturbations for systems of different size (# of states) because of the call to the random number generator within a PSID simulation. This PR will reset the state of the random number generator so the state is not changed during a simulation.

@m-bossart m-bossart requested a review from jd-lara August 29, 2023 18:58
@github-actions
Copy link
Contributor

Performance Results

Version Precompile Time
Main 4.553962067
This Branch 4.770556177
Version Run Time
Main-Build ResidualModel 11.410039083
Main-Execute ResidualModel 30.330661418
Main-Build MassMatrixModel 0.995980207
Main-Execute MassMatrixModel 303.916684105
This Branch-Build ResidualModel 11.396617335
This Branch-Execute ResidualModel 30.338091671
This Branch-Build MassMatrixModel 0.995813355
This Branch-Execute MassMatrixModel 287.937015293

ResidualModel and MassMatrixModel performance results should be compared between versions and not between models due to the execution order of the tests

@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Merging #353 (ad99c83) into main (a51524d) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #353      +/-   ##
==========================================
- Coverage   87.29%   87.23%   -0.07%     
==========================================
  Files          65       65              
  Lines        8986     8990       +4     
==========================================
- Hits         7844     7842       -2     
- Misses       1142     1148       +6     
Flag Coverage Δ
unittests 87.23% <100.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/base/jacobian.jl 96.00% <100.00%> (+0.22%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Member

@jd-lara jd-lara left a comment

Choose a reason for hiding this comment

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

This shouldn't affect the results of the simulation. Can you explain better what you are seeing that this PR is supposed to correct?

@m-bossart
Copy link
Collaborator Author

I would like these two to give the same result:

Random.seed!(1)
print(rand())
Random.seed!(1)
Simulation(MassMatrixModel, sys, pwd(), (0.0,1.0)
print(rand())

@jd-lara jd-lara merged commit 7e64147 into main Aug 29, 2023
9 checks passed
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.

None yet

2 participants