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

Docs mention module EKI.jl but there's only EKP.jl #81

Closed
glwagner opened this issue Nov 2, 2020 · 1 comment
Closed

Docs mention module EKI.jl but there's only EKP.jl #81

glwagner opened this issue Nov 2, 2020 · 1 comment

Comments

@glwagner
Copy link
Member

glwagner commented Nov 2, 2020

image

bors bot added a commit that referenced this issue Feb 4, 2022
122: [WIP] Use EKP.jl 0.2.0 & pass explicit RNG object where used r=odunbar a=tsj5

[*Edited 1 Feb 2022 in response to updates*] 

This PR updates CES to use the [0.2.0 release](https://github.com/CliMA/EnsembleKalmanProcesses.jl/releases/tag/v0.2.0) of EnsembleKalmanProcesses.jl (EKP). In addition to using changed module and method names, the PR adds support in CES for use of an explicit random number generator where needed; this was done in EKP [#81](CliMA/EnsembleKalmanProcesses.jl#81).

[Recommended practice](https://discourse.julialang.org/t/set-the-state-of-the-global-rng/12599/3) for reproducible tests is to pass an RNG object explicitly to all methods which need it. It's not sufficient to set the global seed for the default RNG: the default RNG algorithm used by Julia was [changed](https://julialang.org/blog/2021/11/julia-1.7-highlights/#new_rng_reproducible_rng_in_tasks) in version 1.7. This will affect us in the future when we update CES to support Julia > 1.6 (#121): the [docs](https://docs.julialang.org/en/v1/stdlib/Random/#Reproducibility) for the Random package note that the same seed may give different random values in different versions.

Changes made to the CES API in this PR are minor (which is why the RNG change is combined with the EKP update):
- An `rng` field is added to `MCMC`; the constructor sets this to `Random.GLOBAL_RNG` by default.
- `rng` is recognized as an optional first argument to `get_obs_sample`, used in lieu of the `rng_seed` kwarg if provided.

All changes are fully backwards compatible. In particular, tests are updated to use an explicit rng, but examples have been left unchanged. It may be desirable to update them as well.


Co-authored-by: Thomas Jackson <tom.jackson314@gmail.com>
bors bot added a commit that referenced this issue Feb 15, 2022
122: [WIP] Use EKP.jl 0.2.0 & pass explicit RNG object where used r=odunbar a=tsj5

[*Edited 1 Feb 2022 in response to updates*] 

This PR updates CES to use the [0.2.0 release](https://github.com/CliMA/EnsembleKalmanProcesses.jl/releases/tag/v0.2.0) of EnsembleKalmanProcesses.jl (EKP). In addition to using changed module and method names, the PR adds support in CES for use of an explicit random number generator where needed; this was done in EKP [#81](CliMA/EnsembleKalmanProcesses.jl#81).

[Recommended practice](https://discourse.julialang.org/t/set-the-state-of-the-global-rng/12599/3) for reproducible tests is to pass an RNG object explicitly to all methods which need it. It's not sufficient to set the global seed for the default RNG: the default RNG algorithm used by Julia was [changed](https://julialang.org/blog/2021/11/julia-1.7-highlights/#new_rng_reproducible_rng_in_tasks) in version 1.7. This will affect us in the future when we update CES to support Julia > 1.6 (#121): the [docs](https://docs.julialang.org/en/v1/stdlib/Random/#Reproducibility) for the Random package note that the same seed may give different random values in different versions.

Changes made to the CES API in this PR are minor (which is why the RNG change is combined with the EKP update):
- An `rng` field is added to `MCMC`; the constructor sets this to `Random.GLOBAL_RNG` by default.
- `rng` is recognized as an optional first argument to `get_obs_sample`, used in lieu of the `rng_seed` kwarg if provided.

All changes are fully backwards compatible. In particular, tests are updated to use an explicit rng, but examples have been left unchanged. It may be desirable to update them as well.


Co-authored-by: Thomas Jackson <tom.jackson314@gmail.com>
bors bot added a commit that referenced this issue Feb 16, 2022
122: [WIP] Use EKP.jl 0.2.0 & pass explicit RNG object where used r=tsj5 a=tsj5

[*Edited 1 Feb 2022 in response to updates*] 

This PR updates CES to use the [0.2.0 release](https://github.com/CliMA/EnsembleKalmanProcesses.jl/releases/tag/v0.2.0) of EnsembleKalmanProcesses.jl (EKP). In addition to using changed module and method names, the PR adds support in CES for use of an explicit random number generator where needed; this was done in EKP [#81](CliMA/EnsembleKalmanProcesses.jl#81).

[Recommended practice](https://discourse.julialang.org/t/set-the-state-of-the-global-rng/12599/3) for reproducible tests is to pass an RNG object explicitly to all methods which need it. It's not sufficient to set the global seed for the default RNG: the default RNG algorithm used by Julia was [changed](https://julialang.org/blog/2021/11/julia-1.7-highlights/#new_rng_reproducible_rng_in_tasks) in version 1.7. This will affect us in the future when we update CES to support Julia > 1.6 (#121): the [docs](https://docs.julialang.org/en/v1/stdlib/Random/#Reproducibility) for the Random package note that the same seed may give different random values in different versions.

Changes made to the CES API in this PR are minor (which is why the RNG change is combined with the EKP update):
- An `rng` field is added to `MCMC`; the constructor sets this to `Random.GLOBAL_RNG` by default.
- `rng` is recognized as an optional first argument to `get_obs_sample`, used in lieu of the `rng_seed` kwarg if provided.

All changes are fully backwards compatible. In particular, tests are updated to use an explicit rng, but examples have been left unchanged. It may be desirable to update them as well.


Co-authored-by: Thomas Jackson <tom.jackson314@gmail.com>
@odunbar
Copy link
Collaborator

odunbar commented May 27, 2022

Resolved in PR #139

@odunbar odunbar closed this as completed May 27, 2022
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

No branches or pull requests

2 participants