Make corrXY work with multiple GEN_KWs

[dafeda]

Introduction

A while back we introduced a call-back to the iterative_ensemble_smoother to allow fetching the cross-correlation matrix between parameters and responses.
This matrix was saved into ert's internal storage as corr_XY.nc and the load_cross_correlations API end-point was created.
The idea was to provide users with the raw data and let them analyse and visualise it as they saw fit.
This feature was not clearly communicated to our users and was therefore not used at all.
Also, it turns out that the implementation is buggy in that it overwrites existing data such that only the GEN_KW that was specified last in ert's config is available.
Works for the poly-case, but not for configs with multiple GEN_KW's.

Approach

Save cross correlations to a parquet file with the following schema:

The obs_name is created by appending a suffix to obs_group. The suffix is a counter that depends on the number of observations within each group.
Data is stored in "tidy format" to make it easy for users to query the data however they want.

Testing using the poly-case (see Plot_correlations.ipynb in the poly_example test folder)

Note that the poly-case has just one GEN_KW.

Testing using the heat equation (see Plot_correlations.ipynb in the heat_equation test folder)

To test with multiple GEN_KWs, I've added two GEN_KWs to the heat-equation, and created a Jupyter Notebook that plots correlations for all iterations. See diff for all plots. Here's an example of one of the parameters at iteration 0:

Testing using Drogon

The FMU-research team has done testing using Drogon.

Here's an example I created using Drogon:

• PR title captures the intent of the changes, and is fitting for release notes.
• Added appropriate release note label
• Commit history is consistent and clean, in line with the contribution guidelines.
• Make sure unit tests pass locally after every commit (git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"')

When applicable

• When there are user facing changes: Updated documentation
• New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
• Large PR: Prepare changes in small commits for more convenient review
• Bug fix: Add regression test for the bug
• Bug fix: Create Backport PR to latest release

[xjules]

There are some failing tests though.

[codspeed-hq]

CodSpeed Performance Report

Merging #9426 will degrade performances by 11.04%

_{Comparing dafeda:heat-equation-gen-kws (5ad84b8) with main (a6d2dba)}

Summary

❌ 1 regressions
✅ 23 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	dafeda:heat-equation-gen-kws	Change
❌	test_direct_dark_performance_with_storage[gen_x: 20, sum_x: 20 reals: 10-gen_data_with_obs-get_record_observations]	2.3 ms	2.6 ms	-11.04%

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.73%. Comparing base (d82fc01) to head (5ad84b8).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9426      +/-   ##
==========================================
+ Coverage   82.69%   91.73%   +9.03%     
==========================================
  Files         426      426              
  Lines       26559    26566       +7     
==========================================
+ Hits        21964    24371    +2407     
+ Misses       4595     2195    -2400     

Flag	Coverage Δ
cli-tests	39.72% <100.00%> (?)
everest-models-test	34.14% <21.42%> (-0.01%)	⬇️
gui-tests	71.86% <85.71%> (?)
integration-test	38.03% <21.42%> (-0.01%)	⬇️
performance-tests	51.63% <28.57%> (-0.01%)	⬇️
test	39.40% <21.42%> (-0.01%)	⬇️
unit-tests	74.18% <28.57%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[oyvindeide]

Would it be possible to use the index for the observations here instead? I guess it is ok for summary observations, though perhaps a bit confusing as this number will not align with report_step, but for GEN_OBS which already have an int index it would probably not align with that, and could be confusing.

[dafeda]

Not sure. Do I remember correctly that the index is a time-stamp or date for summary observations? We could perhaps still use it but perhaps a bit messy.

[oyvindeide]

Yes, was thinking a bit more about this, and should we consider doing it like we do for the snapshot in the gui showing the scaling factors when doing auto scaling? Those events are automatically written to csv by the client, so will be available to the user.

Something like:

ert/src/ert/analysis/_es_update.py

Line 239 in 7127528

AnalysisDataEvent(

, which alread uses the indices:

ert/src/ert/analysis/_es_update.py

Line 252 in 7127528

indexes[obs_group_mask],

[dafeda]

As far as I know, these are the two types of indexes we have:

Are you suggesting we use the second part of the string as an index?

[oyvindeide]

Are you suggesting we use the second part of the string as an index?

Yes, or perhaps send the whole thing as an event instead of saving it in storage. Then everyone gets the benefit right away.

[dafeda]

Not sure I follow the event thing. Would we then still be able to analyse the data from a Jupyter Notebook?

[oyvindeide]

This might be a bit confusing if we just merge with existing data?

[dafeda]

What do you find confusing?

[oyvindeide]

If there is something on disk, we merge with that? Couldnt that potentially be done with a different threshold, and so no longer valid? I might be misunderstanding.

[dafeda]

Good point. self.mount_point points to a specific ensemble. Can we run the same ensemble with different thresholds?