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

Use source model SATNUM regions when generating a FlowNet #188

Merged
merged 13 commits into from
Sep 17, 2020
Merged

Use source model SATNUM regions when generating a FlowNet #188

merged 13 commits into from
Sep 17, 2020

Conversation

olelod
Copy link
Collaborator

@olelod olelod commented Sep 15, 2020 •
edited
Loading

PR #160 closed using EQLNUM regions from source model, this PR deals with using source model SATNUM regions.

For models where many flow tubes end up outside of the source model simulation grid, looping through all cells to find the closest region can be time consuming. Changed here to find the closest flow tube allready assigned a region, which is much faster (and close to the same).

Contributor checklist

  • 🎉 This PR closes the last part of High degrees of freedom with individual satnum/eqlnum #139
  • 📜 I have broken down my PR into the following tasks:
    • Extract SATNUM regions from input simulation and assign regions to flow tubes
    • Testing
  • 🤖 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.
  • 📚 I have considered updating the documentation.

@olelod olelod self-assigned this Sep 17, 2020
@olelod olelod added the enhancement New feature or request label Sep 17, 2020
@anders-kiaer anders-kiaer marked this pull request as ready for review September 17, 2020 07:44
wouterjdb
wouterjdb requested changes Sep 17, 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.

Looks good @olelod, I just didn't really get a good understanding of a part of the code. If you could clearify then we can merge 👍

.github/workflows/flownet.yml Outdated Show resolved Hide resolved
src/flownet/ahm/_run_ahm.py Show resolved Hide resolved
anders-kiaer
anders-kiaer requested changes Sep 17, 2020
View reviewed changes
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.

Some minor suggestions - otherwise LGTM 🚀

src/flownet/ahm/_run_ahm.py Outdated Show resolved Hide resolved
src/flownet/ahm/_run_ahm.py Outdated Show resolved Hide resolved
wouterjdb
wouterjdb approved these changes Sep 17, 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 🎉

@olelod olelod merged commit 173695c into master Sep 17, 2020
@olelod olelod deleted the satnum-regions-from-sim branch September 17, 2020 10:58
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

@anders-kiaer anders-kiaer anders-kiaer 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.
3 participants
@olelod @wouterjdb @anders-kiaer