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

Inconsistency between CalcAvailableUnconfinedAquifer and WithdrawGroundwaterIrrigation #595

Closed
billsacks opened this issue Dec 13, 2018 · 0 comments · Fixed by #600
Closed

Inconsistency between CalcAvailableUnconfinedAquifer and WithdrawGroundwaterIrrigation #595

billsacks opened this issue Dec 13, 2018 · 0 comments · Fixed by #600
Assignees
@billsacks
Labels
bug something is working incorrectly

Comments

@billsacks
Copy link
Member

Brief summary of bug

The code for calculating per-layer water availability is inconsistent between CalcAvailableUnconfinedAquifer and WithdrawGroundwaterIrrigation.

General bug information

CTSM version you are using: ctsm1.0.dev020

Does this bug cause significantly incorrect results in the model's science? No

Configurations affected: Only configurations with groundwater irrigation (which is newly available in ctsm1.0.dev020)

Details of bug

In CalcAvailableUnconfinedAquifer:

             if (j==jwt(c)+1) then
                available_water_layer=max(0._r8,(s_y*(zi(c,j) - zwt(c))*1.e3))
             else
                available_water_layer=max(0._r8,(s_y*(zi(c,j) - zi(c,j-1))*1.e3))
             endif

In WithdrawGroundwaterIrrigation:

                irrig_layer=min(irrig_total,(s_y*(zi(c,j) - zwt(c))*1.e3))

It seems incorrect that irrig_layer always has zi - zwt, instead of sometimes having zi(c,j) - zi(c,j-1) as in CalcAvailableUnconfinedAquifer.

I think this problem is related to the more general bug in the calculation of rsub_top_layer that @swensosc initially fixed in #523 but I asked him to back out of that PR (deferring its fix to its own PR). Based on the fix he proposed there, it seems that the code in CalcAvailableUnconfinedAquifer is correct, and the code in WithdrawGroundwaterIrrigation is incorrect.
@billsacks billsacks self-assigned this Dec 13, 2018
@billsacks billsacks added the bug something is working incorrectly label Dec 13, 2018
billsacks added a commit to billsacks/ctsm that referenced this issue Dec 13, 2018
@billsacks
Fix available water calculation in WithdrawGroundwaterIrrigation
f06e8f6 
The code for calculating per-layer water availability was inconsistent
between CalcAvailableUnconfinedAquifer and
WithdrawGroundwaterIrrigation. It seemed incorrect that, in
WithdrawGroundwaterIrrigation, irrig_layer always had zi - zwt, instead
of sometimes having zi(c,j) - zi(c,j-1) as in
CalcAvailableUnconfinedAquifer.

I think this problem is related to the more general bug in the
calculation of rsub_top_layer that @swensosc initially fixed in
ESCOMP#523 but I asked him to back out of that PR (deferring its
fix to its own PR). Based on the fix he proposed there, it seems that
the code in CalcAvailableUnconfinedAquifer is correct, and the code in
WithdrawGroundwaterIrrigation is incorrect.

This commit fixes the issue.

Resolves ESCOMP#595
@billsacks billsacks mentioned this issue Dec 28, 2018
Merged
billsacks added a commit that referenced this issue Jan 8, 2019
@billsacks
Merge branch 'water_tracers_apply_irrig'
21ae80a 
Set tracer version of irrigation fluxes

This required a substantial rewrite of ApplyIrrigation (now renamed to
CalcIrrigationFluxes). The problem was that the code was written to
compute the total irrigation withdrawal from groundwater, then use this
to compute the application fluxes (drip/sprinkler), then later divide
this total withdrawal by layer. But for tracer fluxes to be computed
correctly, we needed to reorder this, so that the per-layer withdrawals
are determined before determining the application fluxes. This is
because the application flux needs to know the tracer concentrations of
the source, which requires knowing which layers the source is drawing
from. This was made more challenging because of the mix of patch-level
and column-level variables at play: irrigation demand is patch-level,
groundwater availability and extraction is column-level, but application
is back to patch-level.

In a somewhat-related change, I also reworked the passing of information
between soil hydrology (groundwater availability) and irrigation:
Previously, there was some near-duplicate code in
CalcAvailableUnconfinedAquifer (which was called prior to
ApplyIrrigation) and WithdrawGroundwaterIrrigation (which was called
after ApplyIrrigation). I have reworked the code to remove this
duplication, calling the new CalcIrrigWithdrawals in the midst of
CalcIrrigationFluxes. In doing so, I reconciled an accidental
discrepancy between the two original routines (see
#595).

Resolves #521 (Set tracer versions of fluxes set by
ApplyIrrigation)

Resolves #593 (Generalize groundwater irrigation availability
to handle multiple patches per column)

Resolves #595 (Inconsistency between
CalcAvailableUnconfinedAquifer and WithdrawGroundwaterIrrigation)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@billsacks billsacks

Labels
bug something is working incorrectly
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set tracer version of Irrigation fluxes billsacks/ctsm
1 participant
@billsacks