Define the process for contributions, pull requests, versioning etc

[andybeet]

I think we should create some guidelines on how to contribute to this repo. For example

• Create a pull request from a development branch with changes
• Fully document the changes made in the pull request
• Test changes and provide instructions on how others should test
• Require multiple reviewers to review the changes
• Set up some standard tests for which the changes must pass before being pulled into the dev branch
• Version the trunk after each pull request has been merged into main
• plus ....

[eafulton]

G’day Thanks Andy. In being cautious the SVN migration ended up at AtlantisFromSVN. I haven’t had a chance to see whether the history came with it, but it was the third attempt and I know not all branches made it - the really early ones prior to 2006 didn’t come across, though I have checked out versions locally. So first question for the group is do we keep the history (AtlantisFromSVN) separate for historical reference purposes and just roll forward with this AtlantisTrunk version as the starting point or do we actually enrol the broader set of Atlantis users on the AtlantisFromSVN repository? Happy to have rules about submission. When it was just Bec and I committing code we could rest assured any new chaos came from me, but with a broader group having some rules to bring me under control and avoid multi-user chaos sounds sensible. We did used to have a test server which was set up to check the code compiled and ran a simple test but CSIRO has killed that multiple times in the last few years and has told me it won’t be possible running forward at least on CSIRO infrastructure. The other thing to keep in mind is that we need to balance code rigour and the need to avoid messing things up painfully and the time taken to code, test, review, merge etc. If you’re professional coders with dedicated project time that’s one thing, it gets tricker to make it rigorous and doable when it’s a “spare time” job in a busy life. My knowledge of the code gives me some time saving but getting time sis till tough. On the flip side with a broader group who can commit changes that can ease up time pressure too. Cheers Beth From: Andy Beet ***@***.***> Reply to: Atlantis-Ecosystem-Model/AtlantisTrunk ***@***.***> Date: Tuesday 10 December 2024 at 5:26 am To: Atlantis-Ecosystem-Model/AtlantisTrunk ***@***.***> Cc: Subscribed ***@***.***> Subject: [Atlantis-Ecosystem-Model/AtlantisTrunk] Define the process for contributions, pull requests, versioning etc (Issue #2) I think we should create some guidelines on how to contribute to this repo. For example * Create a pull request from a development branch with changes * Fully document the changes made in the pull request * Test changes and provide instructions on how others should test * Require multiple reviewers to review the changes * Set up some standard tests for which the changes must pass before being pulled into the dev branch * Version the trunk after each pull request has been merged into main * plus .... — Reply to this email directly, view it on GitHub<#2>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AD3OHER37DQVBAXZIUGEUR32EXODRAVCNFSM6AAAAABTJN6JNCVHI2DSMVQWIX3LMV43ASLTON2WKOZSG4ZDOOJRHEZDAMQ>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[somros]

Hi Andy,

These are great questions. Just to clarify the meaning behind my recent PR, that was to tidy up some code that Beth has recently helped me build. In other words, I did not just jump at the new GitHub repo and started making my own additions, in case this helps clarify :) I am now in the process of testing the new functions.

Beth, thank you for porting to code from the SVN, that really sounds like a pain. I wonder if having the code on GitHub is an opportunity to take some of the load off your shoulders while still allowing users to pull the code and even contributing. But yes, I imagine that some important question are (1) is CSIRO happy to have Atlantis hosted on GitHub and how do they intend to regulate access, and (2) are there limitations to people's contributions that could come from their home institutions (e.g. how IP is dealt with).

I imagine that in an ideal world the burden of testing new functionalities as well as adding documentation to the user guide should fall on us users? I feel like some folks had already started these conversations a short while back, so now that the SVN croaked for good may be a good time to motivate us as a community to talk again?

[KevCaz]

IMHO, if the group of potential contributors is large, it would be better to protect the main branch (https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/managing-a-branch-protection-rule), but it is not a free feature for private repository.

EDIT: by andybeet. Branch protection rules have been applied to main and dev

[raishalovindeer]

We should probably revisit this issue a bit more urgently and agree on the work flow. I just pulled the trunk from main and it seg faults on my machine. It may have been tested and worked on the Example model, but the example model has multiple versions of .prm files, so it's not clear which files were used to pass the test. There have also now been 70 commits to main since the last release and no update to the versioning, despite Andy's initial suggestion:

Version the trunk after each pull request has been merged into main

I suggest (and I think it was implied in the above) that we use Semantic Versioning for this (MAJOR.MINOR.PATCH) so that we can all keep up.

[andybeet]

@raishalovindeer A few of us attended a mini atlantis workshop several weeks ago. We established guidelines for contributing and collaboration. We started to implement some of this. What we haven't completed yet, is to add these guidelines to the CONTRIBUTING.md which we will do soon after a few kinks have been resolved. As of the workshop the guidelines state, (and the repo has settings) that prevent committing directly to main. All code changes have to be on a branch which merges into dev, then into main. And each PR requires review. The versioning, is going to be non standard (not Semantic versioning). We have some fixes/enhancements in dev right now that will be merged with main soon with a new release. dev has been tested on several atlantis models other than SETAS without error. (Maybe as a test, you could clone the repo from the dev branch and see if you still get segmentation faults)

Regarding the segmentation fault, we now have issue templates for users to submit bugs. I'd ask you fill one out, if you haven't already. It will help us troubleshoot

Regarding multiple automated workflows: It is difficult to test all model configurations in automated workflows because of:

• The vastly different number of model variations
• The need to duplicate many of the parameter files.

The idea of the single model test was a first cut. I think we'd welcome additional variations for specific models, especially if those models are likely to be affected by specific code development. To see which parameter files are currently being used in the workflow you can view the workflow directly

However, for alternative model tests, we'd need SETAS based version of new/ alternate versions of prms. So a working contaminants prm that integrates with the SETAS model, would be needed if your model is running contaminants

[hmorzaria]

@raishalovindeer making note here that I have been testing all changes to the contaminant code in your Salish Sea model and in the Puget Sound Model concurrently. I noted back in January that I was getting a segmentation fault in the Salish Sea model due to a flux error in model box 106. I tried modifying the mixing parameters in the BGM file and relaxing the flux tolerance, but neither helped. As @andybeet mentioned, everything gets reviewed in the SETAS model, and for contaminant changes, @cecilieha ran changes that went into main using her NOBAS model. So I think the segmentation fault is a very specific thing to the Salish Sea model at this point