search

amanzi / ats

Advanced Terrestrial Simulator (ATS) development
Other
47 stars 35 forks source link

improve reliability of ATS development cycle and testing #10

Open ecoon opened 5 years ago

ecoon commented 5 years ago
ecoon commented 5 years ago

Progress on this has been made, and @dasvyat , your help in looking at these would be appreciated. I've added a new repo:

https://github.com/amanzi/ats-regression-tests

which includes the regression test suite, including gold files, and updated ats-demos to NOT include gold, and clear out a bunch of stuff with better documentation.

Daniil, please take a look at these, and then we can start to add things back in. I've deleted a lot of stuff just so you can see where we're at for now. As you add things to demos, please add ipython notebooks as well which plot and describe the capability being shown.

ecoon commented 4 years ago

This is addressed by https://github.com/amanzi/amanzi/pull/364 as the "tag" in ATS which corresponds to the version of ATS which last built with Amanzi will be saved via the submodule (which acts on commits, not branches). But it does not define the last commit in Amanzi that worked against ATS...

ecoon commented 3 years ago

The more I think of this, the more I think that I did the submodule dependency in the wrong direction. If ATS has a submodule pointing at Amanzi, then Amanzi develop can continue to move forward and ATS will still point to the last commit that was working. Updates to Amanzi can be incorporated via "git checkout master; git pull" in the submodule for Amanzi, and once working, ATS can commit the moved pointer.

This will avoid the many seemingly arbitrary commits to Amanzi that simply move the ATS pointer as we try to keep these identical, and will eliminate all issues of Amanzi changes breaking ATS tests without any notion of what changed (because ATS didn't change at all). Never again would I have to drop everything to fix ATS ASAP thanks to an Amanzi commit. And we can close this ticket because there will be no need for such a tag.

ecoon commented 3 years ago

@jd-moulton After the short course, how much work would it be on you if I flipped the submodule dependency. By this, I mean ATS will have a pointer to Amanzi rather than the other way around.

Upside -- we can always pin the exact Amanzi hash against which ATS is known to build. No more breaking ATS by changing Amanzi. Downside -- Users have to now clone ATS instead of cloning Amanzi, it breaks all ATS's new shiny CI and David has to do (potentially) a lot of work updating those scripts.

lipnikov commented 3 years ago

If CI allows us to switch to a fixed Amanzi tag, the problem is solved.

jd-moulton commented 3 years ago

I don't think adapting the CI workflow will be difficult. It is swapping who's in charge so to speak, but we have all the ingredients now (e.g., Docker image with TPLs), and reasonable understanding of github actions.

To Konstantin's point, if we new the Amanzi tag we needed we could pass this through to the Docker stuff and have the ATS build use this tag. However, this is where I think Ethan's point comes in. The way submodules currently work we don't have a place to store the Amanzi tag, instead we have the ATS tag. So swapping is probably necessary. I suspect this isn't too messy but we'd have to give it a try. It's a shame to do it after the short course - should we consider doing it before?

ecoon commented 3 years ago

I'm not turning this course into a death march. The code is fixed for good reason, we can change things afterwards.

ecoon commented 1 year ago

Leaving this open because it discusses and motivates a swap in submodule order, which is still a maintenance goal.