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

UZF unit specific factors seem to assume a feet-meter conversion #1961

Open
Huite opened this issue Jul 25, 2024 · 0 comments
Open

UZF unit specific factors seem to assume a feet-meter conversion #1961

Huite opened this issue Jul 25, 2024 · 0 comments
Assignees
Labels

Comments

@Huite
Copy link
Contributor

Huite commented Jul 25, 2024

This is all really quite pedantic, but I was going through UfzCellGroup.f90, and I noticed a few things:

  !> @brief Calculate unit specific tolerances
  !<
  subroutine factors(feps1, feps2)
    ! -- dummy
    real(DP), intent(out) :: feps1
    real(DP), intent(out) :: feps2
    real(DP) :: factor1
    real(DP) :: factor2
    !
    ! calculate constants for uzflow
    factor1 = DONE
    factor2 = DONE
    feps1 = DEM9
    feps2 = DEM9
    if (ITMUNI == 1) then
      factor1 = DONE / 86400.D0
    else if (ITMUNI == 2) then
      factor1 = DONE / 1440.D0
    else if (ITMUNI == 3) then
      factor1 = DONE / 24.0D0
    else if (ITMUNI == 5) then
      factor1 = 365.0D0
    end if
    factor2 = DONE / 0.3048
    feps1 = feps1 * factor1 * factor2
    feps2 = feps2 * factor1 * factor2
    !
    ! -- Return
    return
  end subroutine factors

...
    ! -- check for falling or rising water table
    if ((thick - thickold) > feps1) then

...
    ffcheck = (this%surflux(icell) - this%uzflst(this%nwavst(icell), icell))
    !
    ! -- increase new waves in infiltration changes
    if (ffcheck > feps2 .OR. ffcheck < -feps2) then

The following things are a bit remarkable:

  • factor2 is defined twice, first as DONE, then again as DONE / 0.3048 (which is the number of feet per meter).
  • feps1 and feps2 are identical, as both come down to DEM9 * factor1 * factor2
  • factor1 obviously depends on the ITMUNI value (0=undefined, 1=seconds, 2=minutes, 3=hours, 4=days, 5=years)

The result is then used as tolerances for falling/rising water tables (feps1) or infiltration changes (feps2).

At some places in MODFLOW 6, lenuni (1=feet, 2=meters, 3=centimeters, 4=unknown) is used, but not for the UZF package as far as I can tell. I checked the IO docs: there's no mention of time or length units anywhere in the UZF section, unlike the length_conversion and time_conversion entries for the lake package.

It seems to me like the code here wants to define a tolerance of 1e-9 in meter/day, and it assumes the thickness is given in feet (which would then be the general MODFLOW 6 length unit, I guess).
There are a lot more hardcoded tolerances in this module which are compared to time step durations, lengths, speeds, etc., which were probably written with a specific set of units in mind.

It probably doesn't matter much for actual outcomes, since most of these:

  use ConstantsModule, only: DZERO, DEM30, DEM20, DEM15, DEM14, DEM12, DEM10, &
                             DEM9, DEM7, DEM6, DEM5, DEM4, DEM3, DHALF, DONE, &
                             DTWO, DTHREE, DEP20

reflect "a small number".

But the feet/meter thing got me a little confused...

@Huite Huite added the bug label Jul 25, 2024
@Huite Huite changed the title UZF unit specific factors seem to a feet-meter conversion UZF unit specific factors seem to assume a feet-meter conversion Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants