Replace KGOs in CI script?

[dustinswales]

Problem: Having to update the KGOs is somewhat of a bottleneck in the code review process, as it cannot be done by all COSP code managers. Also, it requires us to maintain versioning within the codebase.

Potential Solutions:
1.) Within the CI scripts, run the offline COSP tests twice. Once with master branch (i.e. re-create latest KGO) and once with feature (i.e. new KGO), then compare.
2.) Store CI output (KGO) as GitHub Artifacts and download as part of subsequent workflows.

The former is inefficient but very straightforward.
The latter is more elegant, but requires creating a new CI script to re-create the artifact, since theGH Artifacts only exist for a finite time.

@alejandrobodas This would be straightforward to implement in the CI scripts. Thoughts?

This would supersede #50

[alejandrobodas]

@dustinswales I like the idea, but I see one potential issue. We'll be unaware of changes in the KGOs that are caused by external dependencies, e.g. changes in the ifort compiler. This has happened several times in the past.

[dustinswales]

@alejandrobodas After #147 is merged, we won't have an external dependencies in the containerized CI tests. So maybe we could ditch the KGOs?
@RobertPincus Thoughts?

[RobertPincus]

@dustinswales Your solution 1 essentially says "Any proposed change has to not modify what's currently being produced by main", is that the idea? The efficiency doesn't seem important - building the drivers and running the tests takes < 90 seconds.

Solution 2 could be accomplished by having a Github action that runs on the regular (weekly?) and produces new artifacts.

Solution 3? Modify the container-making code to create KGOs that get stored in the containers themselves. (They aren't very big right?)

One downside of all these approaches is that none catch errors arising from, say, #140 nor alert us when compiler behavior changed in a container update.

[dustinswales]

@RobertPincus Why would compiler behavior change in the container if we don't update the Dockerfile?
In #136, if I understand it correctly, the problem was that the compiler in the container was updated w/o updating the KGO's? The compiler didn't magically change on its own in the container, we just didn't update the COSP KGO's for the software stack update. No?

[RobertPincus]

@dustinswales You're right, of course, the compiler won't change if we don't change the Dockerfile/container. I meant more that new compiler versions might introduce changes that we'd want to know about. As I understand it #136 shows ifx a (now-patched?) weakness in our code.

[RobertPincus]

@dustinswales I guess we could introduce a protocol in which we compare existing and proposed KGOs when we update containers?

[dustinswales]

@RobertPincus Maybe this is more of a working practices issue.
Software stack updates can change the results, and should be reviewed like any other PR. In the past, stack changes were outside the repository (e.g., esm-rad images) and could easily go unnoticed.

Going forward software updates will be tied to source code changes in the COSP dockerfiles. So any change to the stack should be intentional and subject to the PR review process.

[RobertPincus]

@RobertPincus Maybe this is more of a working practices issue. Software stack updates can change the results, and should be reviewed like any other PR. In the past, stack changes were outside the repository (e.g., esm-rad images) and could easily go unnoticed.

Going forward software updates will be tied to source code changes in the COSP dockerfiles. So any change to the stack should be intentional and subject to the PR review process.

Good point. I support the idea.

Which implementation? One wrinkle with 3 is that we would have compiler-specific KGOs for free (though it's non-trivial to get them out of the container to compare them)... although I guess we'd also need to regenerate the containers every time we updated master

[dustinswales]

Option 1 can be done in 5 minutes.
Option 2 requires another script for KGO generation.
Option 3 seems overly complicated, and not sure why you would want the cosp output in the image.

[RobertPincus]

@dustinswales Well, option 3 requires folding including some bits of the CI in the Docker image generation. You'd have to get the input files, build the code, and put the outputs somewhere. It's not a huge drama, maybe there's some advantage to being able to look at older versions... but option 1 also seems fine.

You could add this on top of #147 so the CI passes?

[alejandrobodas]

@dustinswales @RobertPincus thanks for this, trying to catch up with the discussion. Not sure if you've agreed on a preferred solution yet. I tend to lean towards option 2. However, if option 1 is straight forward, we could used that one first and move to a more long-term solution later.

[RobertPincus]

@dustinswales @RobertPincus thanks for this, trying to catch up with the discussion. Not sure if you've agreed on a preferred solution yet. I tend to lean towards option 2. However, if option 1 is straight forward, we could used that one first and move to a more long-term solution later.

@alejandrobodas I'm happy to see any of these solutions. Since this is @dustinswales's idea I will follow his lead. Happy to help where needed. Note that we won't want to merge #147 until we have KGOs that let the ifx CI pass.

[dustinswales]

@alejandrobodas @RobertPincus I haven't forgotten about this.
I will take a look at implementing option 2 tonight.