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

fix(sfr): enforce divflow<=1.0 for divs with cprior type 'FRACTION' #636

Merged
merged 2 commits into from
Feb 17, 2021
Merged

fix(sfr): enforce divflow<=1.0 for divs with cprior type 'FRACTION' #636

merged 2 commits into from
Feb 17, 2021

Conversation

emorway-usgs
Copy link
Contributor

Fixes #635

langevin-usgs
langevin-usgs reviewed Feb 16, 2021
View reviewed changes
Copy link
Contributor

@langevin-usgs langevin-usgs 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. One small comment below, which is trivial, and is intended to get you using the constants in the ConstantsModule

src/Model/GroundWaterFlow/gwf3sfr8.f90 Outdated
! -- if diversion cprior is 'fraction', ensure that 0.0 <= fraction <= 1.0
cp = this%divcprior(ii)
divq = this%divflow(ii)
if (cp == 'FRACTION' .and. (divq < 0.0 .or. divq > 1.0)) then
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably use the constants DZERO and DONE instead of 0.0 and 1.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the pointer @langevin-usgs, have patched and PR is now rerunning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@langevin-usgs I looked at the failure and it occurred with one of the models in modflow6-testmodels.git, specifically one that resides in mf5to6:
mf5to6
Not being sure if the error was anomalous, I tried running the model locally after running the conversion program:
mf5to6_error
It doesn't seem like this error is related to the change made in this PR? Since the first commit on this PR passed, then changing 0.0 -> DZERO in the second commit caused it to fail, I'm wondering if we should try and get it to run again?

@langevin-usgs langevin-usgs merged commit e4f80ad into MODFLOW-USGS:develop Feb 17, 2021
@emorway-usgs emorway-usgs deleted the sfr_offer_new_cprior_error branch February 17, 2021 23:11
Hofer-Julian pushed a commit to Hofer-Julian/modflow6 that referenced this pull request Jul 14, 2022
@emorway-usgs @Hofer-Julian
fix(sfr): enforce divflow<=1.0 for divs with cprior type 'FRACTION' (M…
e07dc83 
…ODFLOW-USGS#636)
Hofer-Julian pushed a commit to Hofer-Julian/modflow6 that referenced this pull request Jul 15, 2022
@emorway-usgs @Hofer-Julian
fix(sfr): enforce divflow<=1.0 for divs with cprior type 'FRACTION' (M…
b109d57 
…ODFLOW-USGS#636)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@langevin-usgs langevin-usgs langevin-usgs left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enforce DIVFLOW bounds when cprior is type FRACTION
2 participants
@emorway-usgs @langevin-usgs