search

m2lines / gz21_ocean_momentum

Stochastic-Deep Learning Parameterization of Ocean Momentum Forcing
MIT License
5 stars 0 forks source link

Address issue 44 and 63 #64

Closed tztsai closed 1 year ago

tztsai commented 1 year ago

To address issue #44, I added a random_state argument to the sample function in src/gz21_ocean_momentum/analysis/utils.py, and a seed_all function in src/gz21_ocean_momentum/utils.py. To address issue #63, I moved net._final_transformation = lambda x: x from the test file tests/models/test_fully_conv_net.py into the __init__ method of FullyCNN.

raehik commented 1 year ago

Thanks for this! Right way forwards on the seeding issue. seed_all doesn't currently get used -- in @CemGultekin1 's fork, torch.random_seed is called early on in trainScript.py:

https://github.com/CemGultekin1/gz21/blob/c4c98c30778e7006a69226d814d36537737a5216/gz21/trainScript.py#L133

We probably want to place a call to seed_all around there. Otherwise, I'm happy with the shotgun approach of trying to seed as much as possible in case it crops up in any deeper calls.

I commented on #63 regarding the model final transformation issue. If you'd make a new branch just for the random seed changes, I'd gladly merge it in by itself (up to you).

tztsai commented 1 year ago

Thanks! I'll make a new branch for the random seeding.

raehik commented 1 year ago

Both changes were split out into smaller PRs -- closing.