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

Add random seed argument for CTA simulations #1092

Merged
merged 2 commits into from Jul 27, 2017

Conversation

Projects
None yet
2 participants
@jjlk
Contributor

jjlk commented Jul 27, 2017

Hi @cdeil,
Okay to merge?

@cdeil cdeil self-assigned this Jul 27, 2017

@cdeil cdeil added the cleanup label Jul 27, 2017

@cdeil cdeil added this to the 0.7 milestone Jul 27, 2017

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jul 27, 2017

Member

Just for context: this is a small change to (hopefully) make the examples at https://github.com/gammapy/icrc2017-gammapy-cta-egal fully reproducible.

In 843868f I've changed the argument name from "seed" to "random_state" for consistency with other functions in gammapy: http://docs.gammapy.org/en/latest/development/howto.html#random-numbers and changed the test for this functionality to assert that really the simulation result always is the same.

@jjlk - I'll merge if tests on travis-ci pass and also update all results files in https://github.com/gammapy/icrc2017-gammapy-cta-egal to have a fixed seed now.

Member

cdeil commented Jul 27, 2017

Just for context: this is a small change to (hopefully) make the examples at https://github.com/gammapy/icrc2017-gammapy-cta-egal fully reproducible.

In 843868f I've changed the argument name from "seed" to "random_state" for consistency with other functions in gammapy: http://docs.gammapy.org/en/latest/development/howto.html#random-numbers and changed the test for this functionality to assert that really the simulation result always is the same.

@jjlk - I'll merge if tests on travis-ci pass and also update all results files in https://github.com/gammapy/icrc2017-gammapy-cta-egal to have a fixed seed now.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jul 27, 2017

Member

I updated the examples in https://github.com/gammapy/icrc2017-gammapy-cta-egal
They now give identical output for me if I re-run (and should give identical output for you if you run the scripts that produce the data files @jjlk ).

Member

cdeil commented Jul 27, 2017

I updated the examples in https://github.com/gammapy/icrc2017-gammapy-cta-egal
They now give identical output for me if I re-run (and should give identical output for you if you run the scripts that produce the data files @jjlk ).

@cdeil cdeil merged commit 7da2559 into gammapy:master Jul 27, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@cdeil cdeil changed the title from Add random seed for CTA simulations to Add random seed argument for CTA simulations Jul 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment