Skip to content

Revert SNAKE_OIL_PARAM_{param} to single param-group - #14017

Open
eilskra wants to merge 2 commits into
equinor:mainfrom
eilskra:snake-oil-param-fix
Open

Revert SNAKE_OIL_PARAM_{param} to single param-group#14017
eilskra wants to merge 2 commits into
equinor:mainfrom
eilskra:snake-oil-param-fix

Conversation

@eilskra

@eilskra eilskra commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #14016

Approach
Reverted from multiple parameter groups back to a single one. "Issue" seems to stem from how the seeding/sampling is done (Link). I guess it since its just that it was a different seed/prior sampling, so it just looked different. Unsure if this is something we care about and/or if this is something that should be reverted.

Pre-revert:

seed = f(global_seed, group_name, param_name)  
       ├─ global_seed : RANDOM_SEED
       ├─ group_name  : SNAKE_OIL_PARAM_OP1
       └─ param_name  OP1_PERSISTENCE 

Post-revert

seed = f(global_seed, group_name, param_name)  
       ├─ global_seed : RANDOM_SEED
       ├─ group_name  : SNAKE_OIL_PARAM <- Now all share the same group again
       └─ param_name  : e.g. OP1_PERSISTENCE 

(I would like to thank Copilot for the illustration)

Visual example
Pre-revert:
Screenshot 2026-07-23 at 16 52 11
Post-revert:
Screenshot 2026-07-23 at 16 51 08

  • 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 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • 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: Add backport label to latest release (format: 'backport release-branch-name')

Seems to be that the seeding changes when SNAKE_OIL_PARAM_BPR rather than just SNAKE_OIL_PARAM
@eilskra eilskra self-assigned this Jul 23, 2026
@eilskra eilskra added the release-notes:skip If there should be no mention of this in release notes label Jul 23, 2026
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#58

@eilskra eilskra mentioned this pull request Jul 23, 2026
9 tasks
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 23, 2026

@berland berland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:skip If there should be no mention of this in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected change in snake_oil-plots after dataset merge

3 participants