NREL-Sienna / InfrastructureSystems.jl

Utility package for Sienna's simulation infrastructure
https://nrel-sienna.github.io/InfrastructureSystems.jl/
BSD 3-Clause "New" or "Revised" License
39 stars 20 forks source link

Remove package-specific RNG #198

Closed daniel-thom closed 3 years ago

daniel-thom commented 3 years ago

Fixes #197

The original fix was intended to workaround UUID clashes in PowerSimulations tests. I tested this with the sim_refactor branch of PowerSimulations and that original issue no longer occurs. Tested with Julia 1.5. We changed the way systems get built (PowerSystemsCaseBuilder), and so perhaps that caused some behavior change.

This change will cause the same sequence of UUIDs to be generated when re-running tests in the same Julia 1.5 session. That is what triggered the original issue. Julia is changing its behavior in 1.6. The function UUIDs.uuid4 no longer uses the GLOBAL_RNG, so that behavior will go away then.

From the Julia source:

The default rng used by `uuid4` is not `GLOBAL_RNG` and every invocation of `uuid4()` without
an argument should be expected to return a unique identifier. Importantly, the outputs of
`uuid4` do not repeat even when `Random.seed!(seed)` is called. Currently (as of Julia 1.6),
`uuid4` uses `Random.RandomDevice` as the default rng. However, this is an implementation
detail that may change in the future.

!!! compat "Julia 1.6"
    The output of `uuid4` does not depend on `GLOBAL_RNG` as of Julia 1.6.
codecov[bot] commented 3 years ago

Codecov Report

Merging #198 (4471fc6) into master (f80be08) will decrease coverage by 2.79%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
- Coverage   75.16%   72.37%   -2.80%     
==========================================
  Files          42       42              
  Lines        2988     2668     -320     
==========================================
- Hits         2246     1931     -315     
+ Misses        742      737       -5     
Flag Coverage Δ
unittests 72.37% <100.00%> (-2.80%) :arrow_down:

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

Impacted Files Coverage Δ
src/internal.jl 70.73% <100.00%> (-5.87%) :arrow_down:
src/utils/assert_op.jl 44.44% <0.00%> (-13.89%) :arrow_down:
src/utils/test.jl 42.30% <0.00%> (-7.70%) :arrow_down:
src/single_time_series.jl 59.74% <0.00%> (-7.29%) :arrow_down:
src/time_series_container.jl 53.65% <0.00%> (-6.76%) :arrow_down:
src/serialization.jl 44.44% <0.00%> (-6.06%) :arrow_down:
src/utils/utils.jl 55.45% <0.00%> (-4.13%) :arrow_down:
src/forecasts.jl 84.37% <0.00%> (-4.00%) :arrow_down:
src/time_series_parser.jl 64.78% <0.00%> (-3.97%) :arrow_down:
... and 27 more