nasa / trick

Trick Simulation Environment. Trick provides a common set of simulation capabilities and utilities to build simulations automatically.
Other
34 stars 19 forks source link

Assimilate koviz #1772

Closed cazlo closed 3 weeks ago

cazlo commented 1 month ago

What

Absorb nasa/koviz source code into nasa/trick using a utility which preserves its git history.

Why

Second solution described for remediation of https://github.com/nasa/koviz/issues/13.

There appears to be tight coupling between nasa/trick and nasa/koviz; trick needs koviz to test and koviz needs trick to get data to visualize. To provide data around this statement, in analysis of the NASA org, I am finding only reference to this code in nasa/trick when searching for string github.com/nasa/koviz:

Expanding the search further to all of public GitHub yields similar results from various forks of nasa/trick.

The tight coupling between the components introduces a potential circular dependency with respect to testing, if they are managed as 2 separate codebases/CI runtimes. This appears to be worked around now by the CI tests of nasa/koviz primarily being smoke-check type functions, as opposed to higher fidelity tests which include coverage analysis.

Merging the 2 codebases would replace the need to download and unzip the nasa/koviz artifact at nasa/trick CI runtime, potentially saving a ROM (rough order of magnitude) of between tens of seconds and minutes in the nasa/trick CI operations.

We could reuse the existing nasa/trick Release process, avoiding the need for the 5 manual steps described in the original feature request.

Having 1 git commit which describes the entire system, as opposed to 2, facilitates deterministic, reproducible CI operations. It helps to avoid situations where a change in nasa/koviz introduces fault into nasa/trick in a released version of nasa/trick. (See also NIST 800-218 PO.3.2)

How

Risks

a. It might be hard to show a unified test coverage report in CI. b. Increase nominal I/O for pulls of nasa/trick. The additional I/O comes from pulling the git history of nasa/koviz and the additional source code. This could potentially be mitigated by use of the --squash feature of git subtree. c. Other private uses of nasa/trick and/or nasa/koviz may be impacted. d. Links to nasa/koviz in Engineering documentation would need to be updated. These links are like distributed across several knowledge share platforms of users of nasa/trick, including individual notes, wikis, emails, etc. To mitigate the image of these 'dead links', before we archive nasa/koviz as step number 5 of the procedure, we could update the README or add a banner or something to point people to nasa/trick/koviz. e. The merge request for step # 4 of the procedure will be very big and hard to review. This is mitigated by splitting out these changes into a separate MR: https://github.com/cazlo/trick/pull/3.

cazlo commented 1 month ago

For CI tests performed, see https://github.com/cazlo/trick/pull/2.

I'd totally understand if y'all wanted to re-create the koviz subtree import process, which is one of the main reasons I described it in detail in the Pull Request description. I do request if y'all go that path, to please cherry pick in the commits I made after that, the changes described at https://github.com/cazlo/trick/pull/3. This way the commits will retain a record of my authorship.

Also no worries if we choose not to incorporate this change, I mostly did this for fun because I like to do these kinds of CI merges and re-orgs. Also I did a few test runs of 0a474ba at an equivalent MR in my fork, and the tests are not exactly conclusive. There appears to be a wide variance in the performance in general of these mac-os CI runners, at least today.

This does appear to be shaving about 1.5 minutes off the Trickops tests, though again the sample-set is limited, with only ~4 CI runs looked at.

keithvetter commented 4 weeks ago

Is resistance futile?

sharmeye commented 3 weeks ago

Thank you for the submission. Unfortunately we are unable to assimilate Koviz directly into Trick for legal reasons. However, we do link to Koviz in our documentation as the preferred alternative to Trick's native data plotting tool. As to your observation that TrickOps has an explicit dependency on Koviz, you are correct but TrickOps is itself not even directly managed by the Trick Core team. We may in the future decide to more clearly separate out TrickOps from Trick itself, but for the time being this is impractical.