Fixed random seed application in Spotpy

[richardarsenault]

Spotpy does not have a mechanism to set a seed for the RNG. We use np.random.seed to fix a seed in numpy before launching Spotpy. However, the paper NB had the seed set after Spotpy initialization, leading to the seed being fixed by spotpy instead, before we set our own seed, leading to it being useless. This has been fixed.

[tlvu]

Le code change me semble correct.

Mais j'ai une question, on dirait que les outputs ont tous été enlevé du nb original. C'est normal? Maintenant on veut les garder?

J'ai rien contre garder les outputs. Je préfère les garder même. Je fais juste remarquer le changement de façon de faire. Est-ce que ça risque de surprendre quelqu'un?

[tlvu]

pourquoi ce code cell n'a pas été exécuté avant?

[richardarsenault]

Oui, on avait tout retiré les outputs mais je pense que c'est correct de les garder car il s'agit d'un NB démo qu'on roule dans l'article, alors ça ne change rien (comparativement aux autres NB dans lesquels on veut que les usagers comprennent mieux les étapes pour les répéter dans leurs propres NB).

Merci de l'avoir souligné par contre!

[tlvu]

test_spotpy_calibration fail 2 fois ! Drole de coincidence que le changement est pour le spotpy (https://github.com/CSHS-CWRA/RavenPy/actions/runs/5697951328/job/15445290895?pr=300)

py39

=========================== short test summary info ============================
FAILED tests/test_calibration.py::test_spotpy_calibration[CanadianShield] - ravenpy.ravenpy.RavenError: Config directory: /tmp/tmpk41gc9_c/c019
CHydroUnit constructor:: HRU 2 has a negative or zero area
= 1 failed, 130 passed, 15 skipped, 1 xfailed, 6 xpassed, 20 warnings in 204.82s (0:03:24) =
py39-linux: exit 1 (212.40 seconds) /home/runner/work/RavenPy/RavenPy> pytest --cov pid=4877
.pkg: _exit> python /home/runner/work/RavenPy/RavenPy/.tox/.tox/lib/python3.9/site-packages/pyproject_api/_backend.py True flit_core.buildapi
  py39-linux: FAIL code 1 (496.72=setup[181.13]+cmd[77.43,25.77,212.40] seconds)
  evaluation failed :( (496.99 seconds)
Error: Process completed with exit code 1.

py310

=========================== short test summary info ============================
FAILED tests/test_calibration.py::test_spotpy_calibration[CanadianShield] - ravenpy.ravenpy.RavenError: Config directory: /tmp/tmpm6d45zyt/c031
CHydroUnit constructor:: HRU 2 has a negative or zero area
= 1 failed, 130 passed, 15 skipped, 1 xfailed, 6 xpassed, 28 warnings in 314.22s (0:05:14) =
Error: Process completed with exit code 1

[richardarsenault]

J'ai vu aussi que ça fail, mais hier quand j'ai fait le test ça avait fait un fail avec 3.9 et 3.10 mais pour des deployment différents (ubuntu-latest et ubuntu-latest/Conda). C'est une erreur que j'ai déjà vue avant aussi mais je ne suis pas certain de ce qui la cause! Ça ne devrait pas être en lien avec le NB qui n'a rien changé au code de Spotpy lui-même...

[Zeitsperre]

I'm honestly not sure what the cause of that error is, but I think it has something to do with asynchronous reading of a netcdf file. Feel free to ignore it, it's well beyond the scope of the PR here.

[tlvu]

It's good for me. I already approved the PR anyways. Agreed even if any fixes are required, it's outside the scope of this PR.

If you want to remove the warnings in the first cell, I think you'll have to refresh this notebook in the current PAVICS env because I see you have os.environ["USE_PYGEOS"] = "0" as in https://github.com/Ouranosinc/pavics-sdi/blob/1c2e995716eed853e5863cb08962e36aaefbffc9/docs/source/notebooks/WFS_example.ipynb#L29 but you still have the warning, which is pretty weird.

For NumbaDeprecationWarning, this should work https://github.com/Ouranosinc/pavics-sdi/blob/1c2e995716eed853e5863cb08962e36aaefbffc9/docs/source/notebooks/eccc-geoapi-xclim.ipynb#L249-L253 (which to code view to see the proper highlighting).

Again, make sure to refresh notebook output in the official current Jupyter env.

[richardarsenault]

OK thanks for the responses!

I just tried re-running the NB in the "current" version of the server ("current-stable-version-py39-230601") and I got the same "PYGEOS" warnings. Anything else I should be doing?

[coveralls]

Pull Request Test Coverage Report for Build 5697951328

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 81.46%

---

Totals
Change from base Build 5603522514:	0.0%
Covered Lines:	3304
Relevant Lines:	4056

---

💛 - Coveralls

[tlvu]

Maybe look at the ordering of at env var? It might have to be done before some other import. Also restart the kernel before each run to have a fresh env.

[richardarsenault]

Changed geopandas import order and this fixed the PYGEOS warning! thanks!