Concrete fields for Parameter and Grid

[swilliamson7]

These are a few updates that Enzyme needs in order to run. Specifically regarding the mutable struct changes, I'm working on understanding why this is needed but without it Enzyme crashes. Let me know if you have questions!

[swilliamson7]

Adding that after this is merged, could a new version of ShallowWaters be released? So that I can test some stuff for a presentation I need to give. I'm guessing that, if it doesn't work like I think it should we could always roll back to the last version

[milankl]

Adding that after this is merged, could a new version of ShallowWaters be released? So that I can test some stuff for a presentation I need to give. I'm guessing that, if it doesn't work like I think it should we could always roll back to the last version

Of course no problem. Happy with mutable but ideally fields should have concrete types, I know with ::Real I didn't follow this rule but Int sounds too restrictive (because it could be a float and it would make sense and work) so should we not just say Float64 for these?

[swilliamson7]

For both of the comments on variable type Int, this was originally just a quick fix to make some fields type stable, Float64 should be just as okay!

How do I release a new version (assuming all the checks pass)?

[milankl]

While we're at it I just removed all abstract fields in Parameter and Grid.

[milankl]

A new release can be triggered on any commit (not pull request) with @JuliaRegistrator register however, we'll also need to first set the new version. The changes here are not breaking but in #175 did we cause a breaking change?

[JuliaRegistrator]

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

[swilliamson7]

The changes here are not breaking but in #175 did we cause a breaking change?

I can't find a good definition of a "breaking change" after looking through this and a couple other sites, but I think this means that there might be compatibility issues or something, like the user would need to update to a different Julia version to use ShallowWaters.jl maybe?

In any case, the single struct change should not have caused this sort of change, later changes to include Enzyme might

[milankl]

A breaking change is a new version that is not backwards compatible with previous versions. For example if we change run_model(a,b,c; g=10) to run_model(a,b,c; gravity=10) that's breaking because every code that was written with the previous version using g would throw an error if the keyword g doesn't exist anymore (because it's now gravity). If you implement similar changes within the code that is not supposed to be used by the user then this is fine. There's also a grey zone depending on the outputs. If f(x) returns a in a previous version but then it returns a, b it's a breaking change as not something of the type of a is returned but a tuple meaning that any function using a wouldn't work as before. However, changing g=10 to g=9.81 I wouldn't consider a breaking change because while the simulation is entirely different it's qualitatively similar, almost statistically similar and definitely returns the same types and sizes of these types.

[swilliamson7]

Got it, that makes sense! Then to answer your original question the single struct modification is definitely not a breaking change

[milankl]

Yes, that's why I have increased the version to v0.5.2 not v0.6 ;)

[milankl]

I've just triggered the release on the commit that merged this pull request into main 7d4296a#commitcomment-141817250

[swilliamson7]

Amazing, thank you! To trigger this was the only thing that needed to change the version line in Project.toml?

[milankl]

No you have to comment on a commit with @ JuliaRegistrator register (remove space between @ and J, adding here to avoid it commenting here again, like it did here #176 (comment)). It's a github bot that checks when it's mentioned somewhere and then pulls the current version over into the Julia general registry!