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.

Sign up for GitHub

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

Jump to bottom

Expose observation uncertainty to users via YAML configuration file #132

Merged
merged 16 commits into from
Aug 26, 2020
Merged

Expose observation uncertainty to users via YAML configuration file #132

merged 16 commits into from
Aug 26, 2020

Conversation

olelod
Copy link
Collaborator

@olelod olelod commented Aug 23, 2020 •
edited by wouterjdb
Loading

Closes #120

@wouterjdb wouterjdb changed the title move obs unc to config yaml Expose observation uncertainty to users via YAML configuration file Aug 24, 2020
@wouterjdb wouterjdb added the enhancement New feature or request label Aug 24, 2020
wouterjdb
wouterjdb requested changes Aug 25, 2020
View reviewed changes
Copy link
Collaborator

@wouterjdb wouterjdb left a comment

Choose a reason for hiding this comment

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

Some comments, mostly on what we already discussed: now that we pass around the config we shouldn't change it into a dict somewhere halfway in the code.

.github/workflows/flownet.yml Outdated
@@ -56,7 +56,7 @@ jobs:
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
TESTDATA_REPO_BRANCH: observation-errors-in-config-yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Merge https://github.com/equinor/flownet-testdata/compare/observation-errors-in-config-yaml and put back to master before merging.

src/flownet/ert/_create_ert_setup.py Outdated Show resolved Hide resolved
src/flownet/ert/_create_ert_setup.py Outdated Show resolved Hide resolved
src/flownet/prediction/_run_pred.py Outdated Show resolved Hide resolved
wouterjdb
wouterjdb requested changes Aug 25, 2020
View reviewed changes
src/flownet/ahm/_assisted_history_matching.py Show resolved Hide resolved
src/flownet/ert/_create_ert_setup.py Show resolved Hide resolved
src/flownet/ert/_create_ert_setup.py Outdated Show resolved Hide resolved
@wouterjdb wouterjdb self-requested a review August 26, 2020 13:50
wouterjdb
wouterjdb approved these changes Aug 26, 2020
View reviewed changes
Copy link
Collaborator

@wouterjdb wouterjdb left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

src/flownet/ahm/_assisted_history_matching.py Outdated Show resolved Hide resolved
@wouterjdb wouterjdb marked this pull request as ready for review August 26, 2020 13:55
@wouterjdb wouterjdb merged commit 4bf7394 into master Aug 26, 2020
@wouterjdb wouterjdb deleted the i120-move-obs-unc-to-config-yaml branch August 26, 2020 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wouterjdb wouterjdb wouterjdb approved these changes

Assignees

@olelod olelod

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove hard-coded observation uncertainties
2 participants
@olelod @wouterjdb