search

CliMA / Oceananigans.jl

🌊 Julia software for fast, friendly, flexible, ocean-flavored fluid dynamics on CPUs and GPUs
https://clima.github.io/OceananigansDocumentation/stable
MIT License
966 stars 190 forks source link

Proposal for sustainable `validation` scripts #3076

Open glwagner opened 1 year ago

glwagner commented 1 year ago

The validation scripts have proved useful for code development since they essentially provide convenient scientific / integration tests which would be hard or expensive to implement in CI, but which are still useful when implementing or evaluating a new feature. They also help us share knowledge and code.

Yet a major problem is that the validation scripts are not tested and therefore most of them are broken, since they aren't updated as Oceananigans syntax changes.

I was talking with @siddharthabishnu and we realized that a possible solution would be to follow the convention used by Flux's "model-zoo": https://github.com/FluxML/model-zoo

With this pattern, every validation "experiment" is a directory that includes a collection of scripts and a Project.toml. The Project.toml indicates the version of Oceananigans. If people want to upgrade the scripts + environment they can submit a PR.

We'd still be informally testing the validation scripts, but hopefully this would make them more useful in the future, especially to new users.

We'll have to select a handful of cases that we want to keep around in the process of transitioning to the new system.

navidcy commented 1 year ago

Yes!

navidcy commented 8 months ago

Adding a Manifest + Project toml files in each validation scripts directory is a good idea! However, if we do it while we are working on them in a PR, then the Manifest comes with an Oceananigans dependency that points to the branch, e.g.,

https://github.com/CliMA/Oceananigans.jl/blob/1c2a6f8752b6425bf30d856f8ba0aa681c0ab818/validation/stokes_drift/Manifest.toml#L1202

and the branch gets deleted after the PR is merged.

So what one would need to do is to make another PR with a Manifest pointing to the specific commit on main or to the next tagged release? Sounds like too much work to be sustainable...?

glwagner commented 8 months ago

Hmm, maybe just compat entries instead of Manifest?

navidcy commented 8 months ago

Doesn't ensure 100% reproducibility but definitely easier to do!

glwagner commented 8 months ago

Do we want to have a goal of 100% reproducibility for the validation cases? I think their main purpose is expository, ie to show advanced usage of the code. Exact reproducibility isn't the highest priority for that purpose --- is it?

navidcy commented 8 months ago

True true. I think a Project.toml with compat entries is good enough!