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

Add brine p #417

Merged
merged 10 commits into from
Jun 29, 2021
Merged

Add brine p #417

merged 10 commits into from
Jun 29, 2021

Conversation

plgbrts
Copy link
Contributor

@plgbrts plgbrts commented Jun 18, 2021

This PR continues PR #410 (add brine support)

This PR adds on top of P#410 the creation of WSALT entries in the HISTORY_SCHEDULE.inc

The flownet-testcases/ norne_salt (PR #50 of flownet-testcases repo) is used as testcase

Contributor checklist

  • [ x] : This PR closes Support for BRINE #381.
  • [ x] I have broken down my PR into the following tasks:
    • The creation of WSALT entries in the HISTORY_SCHEDULE.in
  • [ x] : I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR.
    - flownet-testcases/ norne_salt
  • I have considered adding a new entry in CHANGELOG.md.
  • I have considered updating the documentation.

@olelod olelod added this to In progress 🚧 in FlowNet via automation Jun 25, 2021
Copy link
Collaborator

@olelod olelod left a comment

Choose a reason for hiding this comment

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

You should add an item to the CHANGELOG stating that FlowNet now supports brine simulation.

Other that that it looks nice!

I commented all the places where a potential cumulative vector for salt could be added. I don't know enough about brine simulations to know if it is ever used by someone, but I see the vectors are available in OPM Flow, at least. If it is something that is not used at all we should not add them.

FlowNet automation moved this from In progress 🚧 to Review in progress 👀 Jun 25, 2021
@olelod olelod added the enhancement New feature or request label Jun 25, 2021
@plgbrts
Copy link
Contributor Author

plgbrts commented Jun 25, 2021

You should add an item to the CHANGELOG stating that FlowNet now supports brine simulation.

Other that that it looks nice!

I commented all the places where a potential cumulative vector for salt could be added. I don't know enough about brine simulations to know if it is ever used by someone, but I see the vectors are available in OPM Flow, at least. If it is something that is not used at all we should not add them.

I followed your suggestions to generalise and to allow also for WSPT, WSIT, WSIR, similarly as done e.g. for water production/injection (WWXX).

@plgbrts plgbrts closed this Jun 25, 2021
FlowNet automation moved this from Review in progress 👀 to Done 🏁 Jun 25, 2021
@olelod olelod reopened this Jun 28, 2021
FlowNet automation moved this from Done 🏁 to In progress 🚧 Jun 28, 2021
@olelod olelod self-requested a review June 28, 2021 11:21
Copy link
Collaborator

@olelod olelod left a comment

Choose a reason for hiding this comment

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

Just one small thing in the observations.ertobs.jinja2 template and we should be good to go!

FlowNet automation moved this from In progress 🚧 to Review in progress 👀 Jun 28, 2021
@plgbrts
Copy link
Contributor Author

plgbrts commented Jun 28, 2021

Many thanks for all the good suggestions. I removed the WSIR from WCONHIST.

FlowNet automation moved this from Review in progress 👀 to Reviewer approved 🎉 Jun 28, 2021
@olelod olelod merged commit 0ea8158 into equinor:master Jun 29, 2021
FlowNet automation moved this from Reviewer approved 🎉 to Done 🏁 Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
FlowNet
  
Done 🏁
Development

Successfully merging this pull request may close these issues.

Support for BRINE
3 participants