CI misc improvements

[justinlaughlin]

Changes:

• Generate image diffs (abs and rel)
  • "abs" is the L2 of image diff in rgb space with a fixed scaling factor
  • "rel" is scaled by the max diff found in the images
  • PNGs are saved for "abs" and "rel"
  • An html is also generated using "rel". This lets you interactively zoom/pan to see where the test failed. It can only be viewed with an internet connection (this was done to save filesize)
• Always run all tests (set fail-fast: false)
• Rename artifacts to (hopefully) be less confusing
• Misc refactoring
• Separate unused code

[tzanio]

Should we add this to the checklist in #284?

[justinlaughlin]

I don't think its necessary since it is only features to help with development. I think the other PRs are very close / ready so I don't want to hold up the release. This could be merged in afterwards.

[najlkin]

You may just rerun it, the Mac runners are extra crappy 😄 🤭

[justinlaughlin]

Btw, has anyone seen this error before? I definitely saw it in a test last week (trying to find the job) but in the next test it was fixed and I'm not sure why.

6: *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'API misuse: setting the main menu on a non-main thread. Main menu contents should only be modified from the main thread.'

[najlkin]

It happens randomly, I have seen that before. It is only on Mac 🍎🪱

[justinlaughlin]

You may just rerun it, the Mac runners are extra crappy 😄 🤭

oh, are you're telling me the mac tests are stochastic? 🤣

we should change the tests results to a probability density function instead of [PASS] / [FAIL]

edit: I was half-joking but out of 6 samples so far these are the {failed tests}: {6,10}, {9}, {10}, {10}, {10,20}, {}
I think this is a common issue though actions/runner-images#8642

edit2: I don't see any differences in the runner image between fails/success 🤔... Best solution might just be to re-run a couple of times on failure.

Current runner version: '2.317.0'
Operating System
  macOS
  14.5
  [2](https://github.com/GLVis/glvis/actions/runs/10067769382/job/27832593078?pr=296#step:1:2)3F79
Runner Image
  Image: macos-1[4](https://github.com/GLVis/glvis/actions/runs/10067769382/job/27832593078?pr=296#step:1:4)-arm64
  Version: 20240714.2
  Included Software: https://github.com/actions/runner-images/blob/macos-14-arm[6](https://github.com/GLVis/glvis/actions/runs/10067769382/job/27832593078?pr=296#step:1:7)4/20240714.2/images/macos/macos-14-arm64-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/macos-14-arm64%2F20240[7](https://github.com/GLVis/glvis/actions/runs/10067769382/job/27832593078?pr=296#step:1:8)14.2

edit3: @najlkin I noticed that runs that were using a cache instead of building mfem on macos failed. I tried forcing the mfem-build step for macos and it seems to have worked? We'll see...

edit4: forcing a build did not solve the macos failures😔

[najlkin]

Just for testing, or maybe just attach an example file here so we can see it 😉

[najlkin]

Still no HTML, it was not that bad idea to test it 😄

[justinlaughlin]

Just for testing, or maybe just attach an example file here so we can see it 😉

I changed the glvis/data hash to make an artifact, I'll switch it back later. There is an html file under mesh-explorer

[najlkin]

Ahaa, like that, but why is it so big? It seems it still includes the picture data. I unzippzed only the HTML and it shows the pictures without the PNGs 😅

[justinlaughlin]

Ahaa, like that, but why is it so big? It seems it still includes the picture data. I unzippzed only the HTML and it shows the pictures without the PNGs 😅

It is the bundled source code. There is an option under figure.write_html() called include_plotlyjs='cdn'. That reduces the filesize down to around the size of the images - but then the html can only be used online. We could do that?

[najlkin]

No, I meant to use file://test.nominal....png in source, so no data in HTML. It is not possible?

[justinlaughlin]

No, I meant to use file://test.nominal....png in source, so no data in HTML. It is not possible?

Can we discuss over slack real quick? might be easier

[najlkin]

Maybe we could add here the that enabling of the style checks for PR CIs. Or separate? 🤔

[justinlaughlin]

Maybe we could add here the that enabling of the style checks for PR CIs. Or separate? 🤔

If you want to add it, up to you. Otherwise imo, separate PR. I need to work on another project so if we want to include this in 4.3 I say ship it as-is. I did consider that too

[tzanio]

I just merged master in here, and everything looks good.

Are we ready to merge this?

[najlkin]

LGTM, good work 👍

[tzanio]

Please mention in CHANGELOG