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

AHM analysis plugin #478

Merged
merged 29 commits into from
Nov 25, 2020
Merged

Conversation

mareco701
Copy link
Contributor

This is to add a new plugin to visualize assisted history matching analysis (how much update in parameters observations trigger, and which observations trigger an update of a parameter when all observations are considered)

Contributor checklist

  • 🎉 This PR closes #ISSUE_NUMBER.
  • 📜 I have broken down my PR into the following tasks:
    • Task 1
    • Task 2
  • 🤖 I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR.
  • 📖 I have considered adding a new entry in CHANGELOG.md, and added it if should be communicated there.

@anders-kiaer
Copy link
Collaborator

anders-kiaer commented Oct 28, 2020

Thanks for the PR @mareco701. 🎉 Changed the status of this to "draft" while waiting for things to go 🟢.

Looks like a module is currently missing according to the logs?
ModuleNotFoundError: No module named 'data_analysis_test1'

setup.py Outdated Show resolved Hide resolved
@anders-kiaer
Copy link
Collaborator

Looks good 👍 Changing temporarily these lines

TESTDATA_REPO_OWNER: equinor
# If you want the CI to (temporarily) run against another branch than master,
# change the value her from "master" to the relevant branch name.
TESTDATA_REPO_BRANCH: master
to mareco701 and inc_test_for_ahmanalysis would help in the last part of the review process by automatically testing against your new testdata PR (then we simply change those two lines back before merge).

@anders-kiaer anders-kiaer changed the title Ahmanalysis plugin AHM analysis plugin Nov 5, 2020
@anders-kiaer
Copy link
Collaborator

This looks ready for review now @mareco701 👍 🎉

@anders-kiaer anders-kiaer marked this pull request as ready for review November 5, 2020 11:11
Copy link
Collaborator

@anders-kiaer anders-kiaer left a comment

Choose a reason for hiding this comment

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

Nice plugin @mareco701.

Give path to input data:

Should we remove this input box in the frontend, since it is already defined in the yaml file? In addition asking for file input will not work very well in ☁️ setting. Then you can also simplify the callback logic (everything related to input file changing).

Show default parameters distribution
Show transformed parameters distribution (if available)

Then maybe at plugin startup you already know if the transformed distribution is available, and can hide (or disable/grey out) this option in order to simplify the UI? Since it is binary choice (default or transformed) maybe it can be a check box instead of two radio buttons?

Add a guided tour explaining what the usersees? See

for an example from another plugin. This is basically just adding a dictionary {element_id: "Your description of the element"} and Webviz framework will take it from there.

Filter observations:
Filter parameters:

Maybe they should use e.g. wcc.Select instead of text input box? Both to make it easier to select among other options, and to ensure only valid options are entered (currently writing an non-existing parameter in the parameter filter results in an IndexError). See https://webviz-subsurface-example.azurewebsites.net/parameter-parallel-coordinates for an example using wcc.Select.

webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
webviz_subsurface/plugins/_ahm_analysis_plugin.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@HansKallekleiv HansKallekleiv left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@anders-kiaer anders-kiaer merged commit 5e89806 into equinor:master Nov 25, 2020
VincentNevermore pushed a commit to VincentNevermore/webviz-subsurface that referenced this pull request Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants