Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove BLOCK_OBSERVATION keyword #3732

Merged
merged 2 commits into from
Aug 29, 2022
Merged

Remove BLOCK_OBSERVATION keyword #3732

merged 2 commits into from
Aug 29, 2022

Conversation

dafeda
Copy link
Contributor

@dafeda dafeda commented Aug 10, 2022

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@dafeda dafeda force-pushed the block_obs branch 4 times, most recently from 25dcbd8 to 5b57576 Compare August 10, 2022 18:17
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2022

Codecov Report

Merging #3732 (2d042a8) into main (d20ddc3) will increase coverage by 0.13%.
The diff coverage is 53.84%.

@@            Coverage Diff             @@
##             main    #3732      +/-   ##
==========================================
+ Coverage   63.43%   63.57%   +0.13%     
==========================================
  Files         600      597       -3     
  Lines       45359    44777     -582     
  Branches     4091     4042      -49     
==========================================
- Hits        28774    28467     -307     
+ Misses      15328    15070     -258     
+ Partials     1257     1240      -17     
Impacted Files Coverage Δ
src/clib/lib/config/config_parser.cpp 72.50% <0.00%> (-1.59%) ⬇️
src/clib/lib/enkf/enkf_node.cpp 37.94% <ø> (ø)
src/clib/lib/enkf/obs_vector.cpp 29.07% <ø> (+5.58%) ⬆️
src/ert/_c_wrappers/enkf/__init__.py 100.00% <ø> (ø)
.../_c_wrappers/enkf/enums/enkf_obs_impl_type_enum.py 100.00% <ø> (ø)
src/ert/_c_wrappers/enkf/observations/__init__.py 100.00% <ø> (ø)
..._c_wrappers/enkf/observations/block_data_config.py 57.14% <ø> (-21.43%) ⬇️
src/ert/_c_wrappers/enkf/plot_data/__init__.py 100.00% <ø> (ø)
src/ert/libres_facade.py 79.82% <ø> (+0.17%) ⬆️
src/clib/lib/enkf/enkf_obs.cpp 59.43% <100.00%> (-4.80%) ⬇️
... and 8 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dafeda dafeda force-pushed the block_obs branch 6 times, most recently from ba715c4 to a3eec71 Compare August 12, 2022 12:56
@dafeda dafeda force-pushed the block_obs branch 3 times, most recently from 71f9a86 to f283d3f Compare August 22, 2022 10:19
@dafeda dafeda marked this pull request as ready for review August 22, 2022 11:19
@dafeda dafeda force-pushed the block_obs branch 2 times, most recently from 9004ec7 to 14319f4 Compare August 22, 2022 11:33
@dafeda
Copy link
Contributor Author

dafeda commented Aug 22, 2022

test ert please

@dafeda dafeda closed this Aug 22, 2022
@dafeda dafeda reopened this Aug 22, 2022
@dafeda dafeda force-pushed the block_obs branch 2 times, most recently from 669c688 to a91a467 Compare August 22, 2022 17:26
@dafeda dafeda requested a review from oyvindeide August 23, 2022 05:25
@dafeda dafeda self-assigned this Aug 23, 2022
@dafeda dafeda added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Aug 23, 2022
Copy link
Collaborator

@oyvindeide oyvindeide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really nice! Did you check what happens when (if) someone runs a config with BLOCK_OBSERVATION after this?

@dafeda dafeda added the release-notes:breaking-change Automatically categorise as breaking change in release notes label Aug 23, 2022
@dafeda
Copy link
Contributor Author

dafeda commented Aug 24, 2022

This looks really nice! Did you check what happens when (if) someone runs a config with BLOCK_OBSERVATION after this?

I added

BLOCK_OBSERVATION RFT_2006
{
   FIELD  = PRESSURE;
   DATE   = 2006-10-22;
   SOURCE = SUMMARY;

   OBS P1 { I = 1;  J = 1;  K = 1;   VALUE = 100;  ERROR = 5; };
   OBS P2 { I = 2;  J = 2;  K = 1;   VALUE = 101;  ERROR = 5; };
   OBS P3 { I = 2;  J = 3;  K = 1;   VALUE = 102;  ERROR = 5; };
};

to snake_oil_field/snake_oil.ert and ran ert gui snake_oil.ert, which throws the following.

2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: BLOCK_OBSERVATION not recognized when parsing: snake_oil.ert ---
2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: { not recognized when parsing: snake_oil.ert ---
2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: DATE not recognized when parsing: snake_oil.ert ---
2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: SOURCE not recognized when parsing: snake_oil.ert ---
2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: OBS not recognized when parsing: snake_oil.ert ---
2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: OBS not recognized when parsing: snake_oil.ert ---
2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: OBS not recognized when parsing: snake_oil.ert ---
2022-08-24 11:59:38,108 - res.config - WARNING - ** Warning keyword: }; not recognized when parsing: snake_oil.ert ---

Error message: ensemble_config_init_FIELD: field type: PRESSURE; is not recognized

@dafeda dafeda force-pushed the block_obs branch 4 times, most recently from 2d042a8 to ee3c1f2 Compare August 26, 2022 13:02
@dafeda dafeda changed the title Block obs Remove BLOCK_OBSERVATION keyword Aug 26, 2022
Copy link
Collaborator

@oyvindeide oyvindeide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

BLOCK_OBS is redundant as it can easily be replaced with GENDATA_RFT.

Remove last traces of internal equinor tests as they are either no longer
valid or redundant.
@dafeda dafeda merged commit fe178fd into equinor:main Aug 29, 2022
@dafeda dafeda deleted the block_obs branch August 29, 2022 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:breaking-change Automatically categorise as breaking change in release notes release-notes:maintenance Automatically categorise as maintenance change in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants