fitsdiff and pytest assertion rewriting leads to large memory increases when tests fail

[braingram]

jwst/jwst/regtest/test_niriss_wfss.py

Line 86 in 7c16638

assert diff.identical, diff.report()

is a common pattern for regression tests. By default pytest will rewrite this assert to capture additional information about the failure (beyond the AssertionError and message). This includes capturing a reference to the diff instance that lives throughout the duration of the test session. Since diff contains references to the input files (and other objects, note that this is not specific to stfitsdiff and also applies to astropy fitsdiff):

jwst/jwst/regtest/st_fitsdiff.py

Line 190 in 7c16638

if len(self.a) != len(self.b):

this likely contributed to the recent termination of regtest runs where:

• many failures were introduced by a context change
• these failures caused pytest to hold only large amounts of memory
• memory usage exceeded that available on the worker node
• worker node terminated before information about failures were reported

There are a few options for addressing this including:

• disabling pytest assertion rewriting. This would have minimal or no impact on asserts like the above but would produce less useful information for other asserts.
• modify fitsdiff to not hold so many references.

Taking the above example:

jwst/jwst/regtest/test_niriss_wfss.py

Lines 85 to 86 in 7c16638

	diff = FITSDiff(rtdata.output, rtdata.truth, **fitsdiff_default_kwargs)
	assert diff.identical, diff.report()

fitsdiff could be modified to compute identical and report and release all references to data arrays, input files, etc. This approach would be complicated by other fitsdiff usage (that doesn't just access identical and report) so perhaps this pre-computation and reference release might make sense to capture in some regtest-specific code. Something like:

def assert_output_matches_truth(output, truth, **kwargs):
    diff = FITSDiff(rtdata.output, rtdata.truth, **fitsdiff_default_kwargs)
    identical = diff.identical
    report = diff.report()
    del diff  # not sure if this is 100% needed
    assert identical, report

[braingram]

@stscijgbot

[stscijirahub]

This issue is tracked on JIRA as JP-4341.

[braingram]

I did some testing of this with a branch (slightly modified from it's current state):
https://github.com/braingram/jwst/tree/isolate_fitsdiff
isolating fitsdiff to a function like the above didn't have any profound impacts on memory usage. Here's a graph of a run where all fitsdiffs were set to fail:

There is a large ramp that occurs after pytest finished (~12:10) which coincides with jfrog uploading the failed files to artifactory.

Put another way:

• jfrog will become the memory peak for large numbers of failures, we should find a way to make this not the case
• isolating fitsdiff did not fix the gradual increase in memory usage during the pytest run

I'll close this ticket as it doesn't seem beneficial (from a memory perspective) to isolate fitsdiff.