How to handle performance tests

[jgfouca]

Based on the discussion during the performance team meeting, notes here: https://acme-climate.atlassian.net/wiki/spaces/EP/pages/3922133143/2023-09-18+Performance+Infrastructure+Meeting+Notes

I am still not 100% sure on what our approach should be so I thought it might be helpful to list the features/behaviors that we want for performance tests:

1. In order to save testing time, it would be very nice if we could use some of the existing tests already being run as both "normal" and "performance" tests. This is tricky because, in order to get useful performance data, you have to turn off I/O, which then renders the test useless from the point-of-view of doing baseline comparisons. It would be nice if there were a setting to exclude I/O from performance metrics. This would allow a test to serve as both a performance test and a regular baseline test. @jayeshkrishna , is this possible?
2. If a test is known to be a performance test, CIME and scripts in $component/cime_config/* should automatically configure things for performance testing without requiring the user to make a lot of additional changes through test-mods etc. @amametjanov , it would be helpful if you could describe the things you need to change in order to get useful performance data.
3. In order to implement (2), the component will need to know the current case is a is a performance test. This should be easy if we assume SAVE_TIMING=ON implies performance test. I think the latest CIME (our submodule doesn't have this yet) we can at least rely on SAVE_TIMING being set to ON for performance tests.
4. CIME will need to mark tests with significant performance decreases as FAILs. I think we already have this via the TPUTCOMP test phase, but this CIME ticket PFS test should save timing output to the baseline directory ESMCI/cime#2918 makes me think that maybe more functionality is needed. @billsacks , can you elaborate?
5. By default, our Jenkins python job code is set to ignore TPUTCOMP and MEMCOMP FAILs by default when reporting test statuses to cdash. The user can change this by adding --check-throughput --check-memory to the test script (in E3SM_test_scripts repo) but this is very easy to forget and it looks like almost no jobs are currently using these flags. Again, in the interest of making life easy for users, I think both these checks should be on by default for tests that have SAVE_TIMING on.
6. We want to be able to "bless" performance changes if we think a significant performance change in a test is acceptable. I think this is the one feature we fully understand and @wadeburgess and @jasonb5 already have most of what's needed here in place.

Feel free to edit to add features.

Related issues:

#5885

[jgfouca]

Depending on the answer to 1. and 2. it may not be possible for a test to be both a baseline and performance test. If that's the case, then it raises the question of whether the new test-suite flag perf : True is adding any value and we should instead just use the PFS test type for all perf tests.

[billsacks]

4. CIME will need to mark tests with significant performance decreases as FAILs. I think we already have this via the TPUTCOMP test phase, but this CIME ticket PFS test should save timing output to the baseline directory ESMCI/cime#2918 makes me think that maybe more functionality is needed. @billsacks , can you elaborate?

At least in CESM, and at least last time I checked, the current TPUTCOMP is just based on the overall run times given in the model log files. This often isn't very useful for understanding differences in timings between runs. What I was referring to in ESMCI/cime#2918 was saving the full contents of the timing directory (i.e., the detailed timing results that give min/max/etc. times spent in different blocks of code surrounded by timers). Is this possibly related to what you already do with SAVE_TIMING?

[jgfouca]

Thanks @billsacks , that does sound more useful than the TPUTCOMP check and possibly addresses point (1) above since it presumably would allow us to compare times that don't include I/O.

[rljacob]

Yes if we examine the timing output more closely, we can look at timers that don't include I/O.

Note that turning off I/O would also break any compareTwo test.

Currently, the fail condition is "time taken is outside tolerance from the last saved run". But if repeated PRs are just under the threshold, the model will gradually slow down and never fail a test. The threshold can't be to small because machine variability can lead to false negatives. So we need something outside of CIME that tracks performance trends over a few days or a a week.

Does the new capability in 6 require separate blesses for both performance fails AND baselines fails? When v3 went in, it slowed down the model and changed answers from baselines. The bless for the baselines also blessed the throughput change. In that case, we knew about the slowdown and were going to bless it but other baseline-change blesses could sneak in a performance degradation unless the performance slowdown is caught and requires its own bless.

[jgfouca]

@rljacob , that's a good point regarding (6). The clear intent from my meetings with @sarats is that we want to keep hist and performance baselines conceptually separate.

[jgfouca]

@jasonb5 , see above. This may require a bit more work on bless_test_results than we initially thought. We may also want to take a look at system_tests_common.py and the timing output to see if there are more useful comparisons that can be implemented.