feat(fmi) Expose the cell saturated fraction to the other models#2764
feat(fmi) Expose the cell saturated fraction to the other models#2764Manangka wants to merge 2 commits into
Conversation
mjr-deltares
left a comment
mjr-deltares
left a comment
There was a problem hiding this comment.
It looks clean, generally OK I'd say, but those 2 comments below need some follow up.
| nr = this%dis%get_nodenumber(nu, 0) | ||
| if (nr <= 0) cycle | ||
| this%gwfsat(nr) = this%bfr%auxvar(1, i) | ||
| this%gwfsat_old(nr) = this%bfr%auxvar(1, i) |
There was a problem hiding this comment.
If you want to do it this way, you also have to write sat_old to the budget file, see the sav_sat subroutine in gwf-npf.f90. But technically, sat_old is already in the file though...
| this%fmi%memoryPath) | ||
| call mem_allocate(this%fmi%gwfsat, this%neq, 'GWFSAT', & | ||
| this%fmi%memoryPath) | ||
| call mem_allocate(this%fmi%gwfsat_old, this%neq, 'GWFSAT_OLD', & |
There was a problem hiding this comment.
Note that the data also has to be copied in to the interface model. This is done in GwtGwtConnection.f90, for example, for gwfsat you see there:
call this%cfg_dv('GWFSAT', 'FMI', SYNC_NDS, (/STG_BFR_EXG_AD/))
which copies gwfsat at the stage STG_BFR_EXG_AD.
And, for parallel, this data might not be there. So we also have to add it to the MPI sync. It should be simple, just follow the whereabouts of this variable in VirtualGwtModel:
type(VirtualDbl1dType), pointer :: fmi_gwfsat => null()
and the same for VirtualGweModel. That, and adding a parallel test of course, should get you there.
christianlangevin
left a comment
christianlangevin
left a comment
There was a problem hiding this comment.
I'm not sure this approach is set up to handle changes in compressible storage. Of course, this is often quite small if reasonable values are used for specific storage. Also, as I'm sure you know, a similar approach is used for the advanced transport packages.
| if (this%fmi%igwfstrgsy /= 0) vold = vold + this%fmi%gwfstrgsy(n) * delt | ||
| vsolid = vcell * (DONE - this%porosity(n)) | ||
|
|
||
| vnew = vcell * this%fmi%gwfsat(n) * this%porosity(n) |
There was a problem hiding this comment.
In this recalculation of vnew and vold does it give the same results as the calculations that you replaced? The previous equations calculate based on compressible and pore volume changes. I feel like this new approach does not account for changes in compressible water volume. For a confined aquifer, it looks like vnew and vold will always be the same.
|
@christianlangevin @mjr-deltares I'm putting this PR on a hold for now till our meeting next week |
3 participants
In the Transport and Energy models the saturation,$S_w$ , is required in the mst and est packages.
However these values aren't exposed through the FMI.
Instead they are computed using the gwfstrgss and gwfstrgsy rates.
mst:
$f_{storage} = \frac{ \partial \left(S_w \theta C\right)}{\partial t}$
There are several reasons to switch to using the saturation directly:
Note
After this change we can almost entirely get rid of gwfstrgss and gwfstrgsy in the FMI. It only being used in 1 place in the prt-fmi file.
Checklist of items for pull request
ruffon new and modified python scripts in .doc, autotests, doc, distribution, pymake, and utils subdirectories.fprettifyFor additional information see instructions for contributing and instructions for developing.