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

Add missing mdu keywords (phase 2) #648

Closed
3 tasks done
veenstrajelmer opened this issue Jun 7, 2024 · 5 comments · Fixed by #654 or #670
Closed
3 tasks done

Add missing mdu keywords (phase 2) #648

veenstrajelmer opened this issue Jun 7, 2024 · 5 comments · Fixed by #654 or #670
Assignees
Labels
type: compatibility Changes needed to be compatible with the computational core

Comments

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Jun 7, 2024

Is your feature request related to a problem? Please describe.
After adding all research keywords in #642 and raising an error on all missing keywords in #622, we get some errors with well-known keywords like wrimap_salinity and wrimap_temperature. These are available, but are not written to the dia file for some reason. They are also not documented in the FM user manual.

There might be more keywords for which this applies. Therefore, after merging of the above PRs, check if these mdu's (and corresponding diafiles) can be read:

  • p:\11210334-004-dcsm-fm\models\dflowfm2d-noordzee_0_5nm-j22_6-v1a\computations\2013-2017\A01\DCSM-FM_0_5nm.mdu
  • p:\11210334-004-dcsm-fm\models\dflowfm3d-noordzee_0_5nm-j22_6-v1a\computations\2006-2015\A01\DCSM-FM_0_5nm.mdu
  • p:\archivedprojects\11206813-006-kpp2021_rmm-2d\C_Work\31_RMM_FMmodel\computations\model_setup\run_207\RMM_dflowfm.mdu

Check related issues for more keywords:

Describe the solution you'd like
Add all missing keywords to HYDROLIB-core. And by FM-kernel team: add all added keywords to the FM manual and diafile

Missing keywords from mdu/dia files
Code to test:

from hydrolib.core.dflowfm import FMModel, ResearchFMModel
from pydantic.v1.error_wrappers import ValidationError

mdu_list = [r"p:\11210334-004-dcsm-fm\models\dflowfm2d-noordzee_0_5nm-j22_6-v1a\computations\2013-2017\A01\DCSM-FM_0_5nm.mdu",
            r"p:\11210334-004-dcsm-fm\models\dflowfm3d-noordzee_0_5nm-j22_6-v1a\computations\2006-2015\A01\DCSM-FM_0_5nm.mdu",
            r"p:\archivedprojects\11206813-006-kpp2021_rmm-2d\C_Work\31_RMM_FMmodel\computations\model_setup\run_207\RMM_dflowfm.mdu",
            r"p:\archivedprojects\11206813-006-kpp2021_rmm-2d\C_Work\31_RMM_FMmodel\computations\model_setup\run_207\results\RMM_dflowfm_0001.dia",
            r"p:\dflowfm\projects\2019_3D-modellering\3D_2023\dflowfm3d-noordzee_0_5nm-j22_6-v1a\computations\2006\A01_vertadvtyp4_idens13\DCSM-FM_0_5nm.mdu"
            ]

for file_mdu in mdu_list:
    try:
        mdu = ResearchFMModel(file_mdu, recurse=False)
    except ValidationError as e:
        print()
        print(file_mdu)
        print(f"ValidationError: {e}")

Missing keywords in [Geometry]:

Missing keywords in [Numerics]:

  • jarhoxu
  • wridia_viscosity_diffusivity_limit
  • transportmethod (deprecated according to feat: Raise error for unknown keywords #632 (comment))
  • transporttimestepping (deprecated according to feat: Raise error for unknown keywords #632 (comment))
  • noderivedtypes (from old rmm dia, maybe not relevant anymore)
  • sobekdfm_umin (from old rmm dia, maybe not relevant anymore)
  • sobekdfm_umin_method (from old rmm dia, maybe not relevant anymore)
  • sobekdfm_minimal_1d2d_embankment (from old rmm dia, maybe not relevant anymore)
  • sobekdfm_relax (from old rmm dia, maybe not relevant anymore)

Missing keywords in [Physics]:

  • jadelvappos

Missing keywords in [Wind]:

  • stericcorrection (from old rmm dia, maybe not relevant anymore)

Missing keywords in [Waves]:

  • wavenikuradse (from old rmm dia, maybe not relevant anymore)

Missing keywords in [Output]:

Missing keywords sanity check
Additionally, there was a sanity check that failed, this should be added as a testcase I think:

from hydrolib.core.dflowfm import FMModel, ResearchFMModel
file_mdu = "mdu.mdu"
mdu = ResearchFMModel()
mdu.save(file_mdu)
mdu_new = ResearchFMModel(file_mdu)

Raises:

ValidationError: 2 validation errors for ResearchFMModel
mdu.mdu -> numerics -> __root__
  Unknown keywords are detected in section: 'Numerics', '['testfixedweirs']' (type=value_error)
mdu.mdu -> wind -> __root__
  Unknown keywords are detected in section: 'Wind', '['varyingairdensity']' (type=value_error)

Missing keywords from table MV
Some of the keywords in the table in #632 (comment) are now available as research keywords, but not all of them:

from hydrolib.core.dflowfm import ResearchFMModel

mdu = ResearchFMModel()

# ValueError also when adding research_* prefix
mdu.general.guiversion
mdu.geometry.branchfile
mdu.geometry.onednetworkfile
mdu.geometry.bedlevelfile
mdu.volumetables.usevolumetablesfile
mdu.numerics.jarhoxu
mdu.numerics.jaorgsethu
mdu.numerics.transporttimestepping
mdu.numerics.transportmethod
mdu.numerics.noderivedtypes
mdu.physics.jadelvappos
mdu.physics.effectspiral
mdu.time.autotimestepdiff
mdu.waves.wavenikuradse
mdu.output.wrimap_salinity
mdu.output.wrimap_temperature
mdu.output.wrimap_input_dt
mdu.output.writebalancefile
mdu.output.wrihis_heatflux
mdu.output.s1incinterval

# these keywords are present as research keyword
mdu.geometry.research_pipefile
mdu.geometry.research_shipdeffile
mdu.numerics.research_jasfer3d
mdu.numerics.research_vertadvtypmom3onbnd
mdu.numerics.research_jposhchk
mdu.numerics.research_newcorio
mdu.numerics.research_jaupwindsrc
mdu.numerics.research_eddyviscositybedfacmax
mdu.numerics.research_icoriolistype
mdu.numerics.research_zlayercenterbedvel
mdu.numerics.research_epshstem
mdu.numerics.research_zwsbtol
mdu.numerics.research_horadvtypzlayer
mdu.numerics.research_corioadamsbashfordfac
mdu.numerics.research_drop3d
mdu.numerics.research_fixedweirfrictscheme
mdu.numerics.research_jbasqbnddownwindhs
mdu.numerics.research_logprofkepsbndin
mdu.physics.research_selfattractionloading
mdu.physics.research_soiltempthick
mdu.physics.research_uniffrictcoef1d2d
mdu.physics.research_umodlin
mdu.time.research_timestepanalysis
mdu.time.research_dtfacmax
mdu.wind.research_windhuorzwsbased
mdu.output.research_writepart_domain
mdu.output.research_velocitydirectionclassesinterval
mdu.output.research_timesplitinterval
mdu.output.research_wrirst_bnd
mdu.output.research_writedfminterpretedvalues
mdu.output.research_velocitymagnitudeclasses
@arthurvd
Copy link
Member

arthurvd commented Jun 12, 2024

Here's a detailed list of my recommendations for what to do with each of the abovementioned keywords (work in progress, will update this comment later with the last bits):
Background info, deprecation table A.4 on: https://content.oss.deltares.nl/delft3d/D-Flow_FM_User_Manual_1D2D.pdf#section.A.4

actions:
ADD -> add to normal FMModel class
RESEARCH -> add/keep in ResearchFMModel class
OBSOLETE -> remove from code

  • wridia_viscosity_diffusivity_limit OBSOLETE (couldn't find it back in the code anymore)

  • sobekdfm_umin ADD (Has been renamed, to be added now: Lateral_fixedweir_umin, double, default 0.0, "Minimal velocity treshold for weir losses in iterative lateral 1d2d weir coupling.")

  • sobekdfm_umin_method RESEARCH (Has been renamed, to be added now: Lateral_fixedweir_umin_method, integer, default 0, "Method for minimal velocity treshold for weir losses in iterative lateral 1d2d weir coupling.")

  • sobekdfm_minimal_1d2d_embankment OBSOLETE

  • sobekdfm_relax OBSOLETE

  • stericcorrection OBSOLETE

  • Wrimap_calibration ADD, integer, default 1, "Write roughness calibration factors to map file."

  • mdu.general.guiversion ADD (IF GUI still writes this)

  • mdu.geometry.branchfile OBSOLETE (was to support SOBEK ascii/.ini network input. Is now all supported in the NetFile using UGRID 1D)

  • mdu.geometry.onednetworkfile OBSOLETE (was to support SOBEK ascii/.ini network input. Is now all supported in the NetFile using UGRID 1D)

  • mdu.geometry.bedlevelfile OBSOLETE (See table A.4)

  • mdu.volumetables.usevolumetablesfile

  • mdu.numerics.jaorgsethu

  • mdu.numerics.transporttimestepping OBSOLETE (See table A.4)

  • mdu.numerics.transportmethod OBSOLETE (See table A.4)

  • mdu.numerics.noderivedtypes RESEARCH

  • mdu.physics.effectspiral OBSOLETE (v1.06 (2016-05-16): Removed 1 variable for secondary flow, EffectSpiral as it is given by Espir contained in .mor file)

  • mdu.time.autotimestepdiff OBSOLETE

  • mdu.waves.wavenikuradse OBSOLETE

  • mdu.output.wrimap_salinity KEEP, integer, default 1, "Write salinity to map file." (accidentally missing in .dia, will be solved once stat.out. for map file is finished)

  • mdu.output.wrimap_temperature KEEP, integer, default 1, "Write temperature to map file." (accidentally missing in .dia, will be solved once stat.out. for map file is finished)

  • mdu.output.wrimap_input_dt OBSOLETE (couldn't find it back in the code anymore)

  • mdu.output.writebalancefile OBSOLETE (See table A.4)

  • mdu.output.wrihis_heatflux OBSOLETE->re-ADD, rename to Wrihis_heat_fluxes

  • mdu.output.s1incinterval OBSOLETE (superseded by classmap output)

  • mdu.geometry.research_pipefile RESEARCH

  • mdu.geometry.research_shipdeffile RESEARCH

  • mdu.numerics.research_jasfer3d ADD jasfer3D, integer, default 0, "corrections for spherical coordinate (0: no, 1: yes)"

  • mdu.numerics.research_vertadvtypmom3onbnd RESEARCH, Vertadvtypmom3onbnd, integer, default 0, "Vert. adv. u1 bnd UpwimpL (0: follow javau, 1: on bnd, 2: on and near bnd)"

  • mdu.numerics.research_jposhchk RESEARCH, jposhchk, integer, default: 2, "Check for positive waterdepth (0: no, 1: 0.7dts, just redo, 2: 1.0dts, close all links, 3: 0.7dts, close all links, 4: 1.0dts, reduce au, 5: 0.7dts, reduce au, 6: 1.0dts, close outflowing links, 7: 0.7*dts, close outflowing links)"

START new part July 2
actions:
ADD -> add to normal FMModel class
RESEARCH -> add/keep in ResearchFMModel class
OBSOLETE -> remove from code

  • mdu numerics jadelvappos RESEARCH, bool, default False, "Only positive forced evaporation fluxes(0: no, 1: yes)."
  • mdu.numerics.jarhoxu RESEARCH, int, default 0, "Include density gradient in advection term (0: no(strongly advised), 1: yes, 2: Also in barotropic and baroclinic pressure term, 3,4: Also in vertical advection)."
  • mdu.numerics.research_newcorio RESEARCH, bool, default True, "New standard way of Coriolis term calculation."
  • mdu.numerics.research_jaupwindsrc RESEARCH, bool, default True, "Upwind advection discretization at source sinks (0: no (higher-order), 1: yes, 1st order upwind)."
  • mdu.numerics.research_eddyviscositybedfacmax RESEARCH, float, default 0.0, "Limit eddy viscosity at bed (factor 0.0-1.0 of first layer above)."
  • mdu.numerics.research_icoriolistype RESEARCH, int, default 5, "Coriolis weighting method (0: No Coriolis, 5: default, 3,4: no weights, 5-10: Kleptsova hu/hs, 25-30: Ham hs/hu, odd: 2D hs/hu, even: hsk/huk)."
  • mdu.numerics.research_zlayercenterbedvel RESEARCH, bool, default True, "Reconstruction of center velocity at half closed bedcells (0: no, 1: copy bed link velocities)."
  • mdu.numerics.research_epshstem RESEARCH, float, default 1e-3, "Only compute heat flux + evaporation if depth > epshstem."
  • mdu.numerics.research_zwsbtol RESEARCH, float, default 0.0, "Tolerance for zws(kb-1) at bed."
  • mdu.numerics.research_horadvtypzlayer RESEARCH, integer, default 0, "Vertical treatment of horizontal advection in z-layers (0: default, 1: N/A, 2: sigma-like)."
  • mdu.numerics.research_corioadamsbashfordfac, RESEARCH, float, default 0.5, "Adams-Bashford factor in Coriolis term (0: No/explicit, 0.5d0=Adams-Bashford), only for Newcorio=1."
  • mdu.numerics.research_drop3d, RESEARCH, float, default 1.0, "Waterdepth factor, apply droplosses in 3D if z upwind below bob + 2/3 hu*drop3D."
  • mdu.numerics.research_fixedweirfrictscheme, RESEARCH, int, default: 0, "Fixed weir friction scheme (0: friction based on hu, 1: friction based on subgrid weir friction scheme, 2: without weir (like WAQUA), 3/4: full undisturbed velocity reconstruction)."
  • mdu.numerics.research_fixedweirtopwidth, float, default: 3.0, "Uniform width of the groyne part of fixed weirs"
  • mdu.numerics.research_fixedweirtopfrictcoef, float, default: N/A, "Uniform friction coefficient of the groyne part of fixed weirs"
  • mdu.numerics.research_fixedweirtalud, float, default: 3.0, "Uniform talud slope of fixed weirs"
  • mdu.numerics.research_fixedweirrelaxationcoef, float, default: 0.6, "Fixed weir relaxation coefficient for computation of energy loss."
  • mdu.numerics.research_jbasqbnddownwindhs, RESEARCH, int, default: 0, "Water depth scheme at discharge boundaries (0: original hu, 1: downwind hs)."
  • mdu.numerics.research_logprofkepsbndin, RESEARCH, int, default 0, "3D profile at open boundaries: (0: k-eps, 1: log k-eps inflow, 2: log k-eps in and outflow)."
  • mdu.physics.research_selfattractionloading, RESEARCH, int, default: 0, "Use self attraction and loading (0: no, 1: yes, 2: only self attraction)."
  • mdu.physics.research_selfattractionloading_correct_wl_with_ini, RESEARCH, bool, default: False, "Correct water level with initial water level in self attraction and loading (0: no, 1: yes)."
  • mdu.physics.research_soiltempthick, RESEARCH, double, default 0.0, "Use soil temperature buffer if > 0, e.g. 0.2 (m)"
  • mdu.physics.research_uniffrictcoef1d2d, RESEARCH, double, default: 0.023, "Uniform friction coefficient in 1D2D links (0: no friction)."
  • mdu.physics.research_umodlin OBSOLETE
  • mdu.time.research_timestepanalysis, RESEARCH, bool, default: False, "Write time steps analysis file *.steps (0: no, 1: yes)."
  • mdu.time.research_dtfacmax, ADD (no research!), float, default 1.1, "Max timestep increase factor in successive time steps."
  • mdu.wind.research_windhuorzwsbased, RESEARCH, int, default: 0, "Wind drag hu or zws based (0: hu, 1: zws)."
    END new part July 2
    COMMENT from 3 weeks ago:
  • mdu.output.research_writepart_domain ADD Writepart_domain, int, default 1, "Write partition domain info. for postprocessing (0: no, 1: yes)."
  • mdu.output.research_velocitydirectionclassesinterval ADD VelocityDirectionClassesInterval, float, "Class map's step size of class values for velocity direction"
    mdu.output.research_timesplitinterval
    mdu.output.research_wrirst_bnd
    mdu.output.research_writedfminterpretedvalues
  • mdu.output.research_velocitymagnitudeclasses ADD VelocityMagnitudeClasses, List[float], "Class map's list of class values for velocity magnitudes"

@tim-vd-aardweg
Copy link
Contributor

@arthurvd Could you check the keywords with a ⚠️?

Status Section Keyword Action performed Todo/Notes
General guiversion Added as optional keyword. Done.
Geometry bedlevelfile None needed. Obsolete. Replaced by IniFieldFile.
⚠️ Geometry keepzlay1bedvol to do... Arthur: is this a research or GA keyword? What is the type/default value/description?
Geometry branchfile None needed. Obsolete.
Geometry onednetworkfile None needed. Obsolete.
⚠️ VolumeTables usevolumetablesfile to do... Arthur: is this a research or GA keyword? What is the type/default value/description?
⚠️ Numerics jarhoxu to do... Arthur: is this a research or GA keyword? What is the type/default value/description?
⚠️ Numerics jaorgsethu to do... Arthur: is this a research or GA keyword? What is the type/default value/description?
Numerics wridia_viscosity_diffusivity_limit None needed. Obsolete.
Numerics transportmethod None needed. Obsolete.
Numerics transporttimestepping None needed. Obsolete.
Numerics noderivedtypes Added to research keywords. Done.
Numerics sobekdfm_umin Renamed: Lateral_fixedweir_umin. Moved from research to GA. Done.
Numerics sobekdfm_umin_method Renamed: Lateral_fixedweir_umin_method. Stays in research. Done.
Numerics sobekdfm_minimal_1d2d_embankment None needed. Obsolete.
Numerics sobekdfm_relax None needed. Obsolete.
Numerics jasfer3d Moved from research to GA. Done.
Numerics velocitymagnitudeclasses Moved from research to GA. Done.
Time autotimestepdiff None needed. Obsolete.
⚠️ Physics jadelvappos to do... Arthur: is this a research or GA keyword? What is the type/default value/description?
Physics effectspiral None needed. Obsolete.
Wind stericcorrection None needed. Obsolete.
Waves wavenikuradse None needed. Obsolete.
Output writebalancefile None needed. Obsolete.
Output wrimap_salinity Added as GA keyword. Done.
Output wrimap_temperature Added as GA keyword. Done.
Output wrimap_calibration Added as GA keyword. Done.
Output wrimap_input_dt Changed name to Wrimap_every_dt Done.
Output wrihis_heatflux None needed. Obsolete.
Output s1incinterval None needed. Obsolete.

@arthurvd
Copy link
Member

arthurvd commented Jul 2, 2024

Hi @tim-vd-aardweg

Geometry keepzlay1bedvol to do... Arthur: is this a research or GA keyword? What is the type/default value/description?

Add: geometry.research_keepzlay1bedvol RESEARCH, bool, default: False, "Correct volumes when keepzlayeringatbed=1 (0: too large bedcell volumes, 1: correct bedcell volumes)."

VolumeTables usevolumetablesfile to do... Arthur: is this a research or GA keyword? What is the type/default value/description?

Add these 3:

  • volumeTables.usevolumetables ADD bool, default: False, "Use 1D volume tables (0: no, 1: yes)."
  • volumeTables.increment ADD float, default: 0.1, "Desired increment for volume tables."
  • volumeTables.usevolumetablesfile ADD bool, default: False, "Use volume table file (0: no, 1: yes)"

Numerics jarhoxu to do... Arthur: is this a research or GA keyword? What is the type/default value/description?

See comment above: mdu.numerics.jarhoxu RESEARCH, int, default 0, "Include density gradient in advection term (0: no(strongly advised), 1: yes, 2: Also in barotropic and baroclinic pressure term, 3,4: Also in vertical advection)."

Numerics jaorgsethu to do... Arthur: is this a research or GA keyword? What is the type/default value/description?

OBSOLETE

Physics jadelvappos to do... Arthur: is this a research or GA keyword? What is the type/default value/description?

See comment above: mdu numerics jadelvappos RESEARCH, bool, default False, "Only positive forced evaporation fluxes(0: no, 1: yes)."

@tim-vd-aardweg
Copy link
Contributor

tim-vd-aardweg commented Jul 4, 2024

Thanks Arthur!! I have added/changed a couple more keywords based on your information. The full table for this issue can be found below:

Status Section Keyword Action performed Todo/Notes
General guiversion Added as optional keyword. Done.
Geometry bedlevelfile None needed. Obsolete. Replaced by IniFieldFile.
Geometry keepzlay1bedvol Added to research. Done.
Geometry branchfile None needed. Obsolete.
Geometry onednetworkfile None needed. Obsolete.
VolumeTables usevolumetablesfile None needed No action needed. There was a typo in this issue. usevolumetablefile is already in the code.
Numerics jarhoxu Added to research. Done.
Numerics jaorgsethu None needed. Obsolete.
Numerics wridia_viscosity_diffusivity_limit None needed. Obsolete.
Numerics transportmethod None needed. Obsolete.
Numerics transporttimestepping None needed. Obsolete.
Numerics noderivedtypes Added to research keywords. Done.
Numerics sobekdfm_umin Renamed: Lateral_fixedweir_umin. Moved from research to GA. Done.
Numerics sobekdfm_umin_method Renamed: Lateral_fixedweir_umin_method. Stays in research. Done.
Numerics sobekdfm_minimal_1d2d_embankment None needed. Obsolete.
Numerics sobekdfm_relax None needed. Obsolete.
Numerics jasfer3d Moved from research to GA. Done.
Numerics velocitymagnitudeclasses Moved from research to GA. Done.
Numerics jadelvappos Added to research. Done.
Time autotimestepdiff None needed. Obsolete.
Time dtfacmax Moved from research to GA. Done.
Physics jadelvappos Added to research. Done.
Physics effectspiral None needed. Obsolete.
Wind stericcorrection None needed. Obsolete.
Waves wavenikuradse None needed. Obsolete.
Output writebalancefile None needed. Obsolete.
Output wrimap_salinity Added as GA keyword. Done.
Output wrimap_temperature Added as GA keyword. Done.
Output wrimap_calibration Added as GA keyword. Done.
Output wrimap_input_dt Changed name to Wrimap_every_dt Done.
Output wrihis_heatflux None needed. Obsolete.
Output s1incinterval None needed. Obsolete.
Output writepart_domain Added to GA. Done.
Output velocitydirectionclassesinterval Added to GA. Done.

@veenstrajelmer
Copy link
Collaborator Author

veenstrajelmer commented Jul 5, 2024

After the implementation in #654, there are still some errors generated by hydrolib core:

  • two keywords are in the FM code, but not yet in hydrolib-core. These were incorrectly marked as deprecated AFAIK
  • several keywords are deprecated, it would be nice to mention this separately, follow-up issue created in Consider recognizing deprecated keywords #668

Missing keywords

  • jadelvappos (int, 0, research, "Only positive forced evaporation fluxes") (from src/dflowfm_data/m_physcoef.f90) >> typo in alias (single p)
  • umodlin (float, 1.0, research, "Linear friction umod, for ifrctyp=4,5,6") (from src/dflowfm_data/m_physcoef.f90)

@priscavdsluis priscavdsluis added the type: compatibility Changes needed to be compatible with the computational core label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: compatibility Changes needed to be compatible with the computational core
Projects
Status: Done
4 participants