Implement DistanceESMDA as a feature flag in the analysis_ES#12690
Implement DistanceESMDA as a feature flag in the analysis_ES#12690xjules merged 2 commits intoequinor:mainfrom
Conversation
3269b68 to
b737928
Compare
f2bbbec to
04b398e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12690 +/- ##
==========================================
- Coverage 90.67% 90.64% -0.04%
==========================================
Files 437 437
Lines 30376 30424 +48
==========================================
+ Hits 27544 27577 +33
- Misses 2832 2847 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| alpha=1, | ||
| seed=rng, | ||
| ) | ||
| T = smoother_es.compute_transition_matrix(Y=S, alpha=1.0, truncation=truncation) |
There was a problem hiding this comment.
Are we calling a method from the wrong object here perhaps?
| T = smoother_es.compute_transition_matrix(Y=S, alpha=1.0, truncation=truncation) | |
| T = smoother_distance_es.compute_transition_matrix(Y=S, alpha=1.0, truncation=truncation) |
There was a problem hiding this comment.
Or is this just for updating scalar parameters later...
If so, needs at least a comment.
There was a problem hiding this comment.
Are we missing np.fill_diagonal(T, T.diagonal() + 1) here?
I see that it's added under the else statement.
There was a problem hiding this comment.
I have updated the code and now it reuses the "else" path and hence np.fill_diagonal(T, T.diagonal() + 1) is present
|
Nitpicking, but we should update the log message to say something about distance based as well: log_msg = (
f"Updating {np.sum(non_zero_variance_mask)} parameters "
f"{'with' if module.localization else 'without'} "
f"adaptive localization."
) |
| obs_main_range: npt.NDArray[np.float64] | None = None | ||
| S_with_loc: npt.NDArray[np.float64] | None = None | ||
| smoother_distance_es: DistanceESMDA | None = None | ||
| if module.distance_localization: |
There was a problem hiding this comment.
I remember discussions where we talked about using distance based for fields and surfaces, and adaptive for scalars.
Has there been a change of plans?
There was a problem hiding this comment.
That will be the goal. But we will start with esmda as discussed.
a1f51b6 to
16ef223
Compare
| from iterative_ensemble_smoother.experimental import AdaptiveESMDA, DistanceESMDA | ||
| from iterative_ensemble_smoother.utils import ( | ||
| calc_rho_for_2d_grid_layer, | ||
| transform_local_ellipse_angle_to_local_coords, |
There was a problem hiding this comment.
I suggest we import transform_local_ellipse_angle_to_local_coords and transform_positions_to_local_field_coordinates from field_utils and remove them from iterative_ensemble_smoother.
There was a problem hiding this comment.
Hm, this is confusing as we had firstly in both places and we decided to have it only in iterative_ensemble_smoother. We do have tests (in test_experimental.py) that makes use of it.
There was a problem hiding this comment.
It's present in ert, so remove! 👍
f314665 to
bc8992f
Compare
| 1,9,7,0,1.5647084,0.69982445,0.8017913,1.2589769,0.681789,0.784527,2.8350534,0.77356935,1.2028819,2.630652,2.49744,1.5107075,0.472015,0.37908825,1.884119,2.620878,0.66226166,1.5549803,3.2782624,0.7100076,3.8008344,2.18281,1.3914294,1.2424315,1.7655207,3.260432,0.86385095,1.9892439,5.720166,1.5746828,0.9667673,2.057492,3.165485,3.572707,2.0172176,1.1081215,1.6948167,0.3319209,1.895517,2.8307996,8.303727,0.4408384,0.7194725,0.3880185,1.3957176,1.2359624,1.0243714,0.21752517,5.378561,2.016633,2.832391,1.2648689,0.23985855,3.6183627,0.9772548,4.4350524,1.8211497,3.1615398,1.1037617,1.4048483,1.0955738,0.0023409685,2.353547,2.5109909,4.6105,1.9041725,1.0138841,1.0030799,1.6351105,2.7451365,2.0925694,1.2776493,3.7343678,-0.10343316,2.25449,3.0830178,0.90670645,1.035188,6.085933,0.39827877,0.9256358,3.4544916,0.46015647,1.2394342,1.8849868,1.1309735,2.2374625,1.551357,7.653069,1.0678707,2.3521543,4.5647106,0.66116333,2.674745,1.4011517,4.8280487,1.5906988,1.0516316,1.9020716,0.3886744 | ||
| 1,9,8,0,1.4976304,0.9155114,1.1456815,1.6042451,0.41697806,1.0752221,3.8301475,0.95881975,1.0762694,3.5554192,3.4360235,1.7755953,0.5781691,0.95956403,2.9440002,2.534437,0.90906817,2.2573998,4.1358433,1.2045456,5.087397,2.3422768,1.9145691,1.7515436,2.179967,4.0263734,1.0011795,1.7008171,5.5737967,1.6201979,1.1530402,1.4989617,4.133435,3.9898384,2.5979807,1.2318301,1.8977118,0.74926955,2.624051,3.7656798,16.043562,0.8338875,1.2254587,0.4745227,1.2867235,2.4197023,1.2448865,0.10686773,6.856317,3.1014512,3.6922688,1.8988659,0.4460432,3.4485896,1.360404,8.417121,2.286479,3.7287915,1.5492749,1.5156914,1.382481,-0.29743764,2.6776562,3.7406878,6.2405353,1.9765283,0.664326,1.2288971,1.8965098,2.3891652,2.1429272,1.6350182,3.3499005,0.42377427,2.3861556,3.5693688,1.2894572,0.92156154,12.956501,0.79943866,1.2738508,5.1443467,1.0212934,2.1932828,2.4608455,0.9701867,2.1765752,2.4519296,9.033463,1.1613481,1.830259,4.6073112,0.8187879,2.3129413,1.639931,4.6843305,1.9804258,1.2324831,2.7098308,0.76770306 | ||
| 1,9,9,0,1.6518232,1.194978,1.4080702,1.7229522,0.3725001,1.2916119,4.765118,1.1652522,1.1434858,5.066947,4.4399576,2.1741774,0.716203,1.501369,3.9145114,2.360794,1.1816856,2.5880527,4.7034864,1.9199827,6.150619,2.4486008,2.4055817,2.1883466,2.5116904,4.3506575,1.2975767,1.551955,4.92699,1.5629336,1.3845143,1.3012034,4.992195,3.2958128,3.0924418,1.4622201,2.0950487,1.1072727,3.289575,4.365122,18.3694,1.1491152,1.6945112,0.6614841,1.2785404,5.461836,1.4692854,0.060847852,8.012202,4.3794227,3.9628088,2.8825338,0.7250154,3.5035584,1.583912,12.417735,2.4141204,4.1771812,2.164907,1.6686194,1.7794459,-0.37494174,2.7779212,4.9685607,6.7366743,2.3456008,0.38613018,1.4809108,2.0311809,2.2412179,2.0741339,1.9614334,3.0306373,1.0402228,2.7045617,3.3241239,1.95023,0.90350676,18.792656,1.3028363,1.6238297,5.846471,1.5711949,3.1383836,2.7591252,1.0428876,1.9512796,3.6340215,6.440026,1.2536687,1.7401216,4.551534,0.91985404,1.9395825,2.0606086,3.641689,1.931977,1.1949116,3.1659126,1.0824698 | ||
| iteration,x,y,z,0,1,10,11,12,13,14,15,16,17,18,19,2,20,21,22,23,24,25,26,27,28,29,3,30,31,32,33,34,35,36,37,38,39,4,40,41,42,43,44,45,46,47,48,49,5,50,51,52,53,54,55,56,57,58,59,6,60,61,62,63,64,65,66,67,68,69,7,70,71,72,73,74,75,76,77,78,79,8,80,81,82,83,84,85,86,87,88,89,9,90,91,92,93,94,95,96,97,98,99 |
There was a problem hiding this comment.
Do we know why "2,3,4,5,6,7,8,9" columns are removed here?
There was a problem hiding this comment.
no idea, hm 🤔
not removed just interleaved differently: 19,2,20 ... 29,3,30 etc
| from tests.ert.ui_tests.cli.run_cli import run_cli | ||
|
|
||
|
|
||
| def assert_variance_in_field( |
There was a problem hiding this comment.
I think I would just inline this code instead of creating a function. But both work fine.
There was a problem hiding this comment.
I decided to keep it as it is easier to understand that both test cases are testing the same.
dafeda
left a comment
There was a problem hiding this comment.
I think this is a good start, well done 👍
In order to make the observations with localization attributes we need to migrate the gen_obs and gen_data into summary observations and summary responses. This includes updating the snapshots and the figure due to the order of observations actually changes.
A new attribute, distance_localization in ESSettings, is introduced that when set makes updating fields and surfaces via the ies.DistanceESMDA module. To set the flag: ANALYSIS_SET_VAR STD_ENKF DISTANCE_LOCALIZATION TRUE This commit includes observations_loc.txt for snake_oil_field and heat_equation cases, where both are tested in test_distance_localization.
|
Should we mention this new functionality in the docs aswell?
|
No, not yet. |
Issue
Resolves #12655
Approach
distance localization will be activated via
Currently supports only SUMMARY_OBSERVATION
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests')When applicable