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 to FMModel class #571

Closed
veenstrajelmer opened this issue Oct 23, 2023 · 5 comments · Fixed by #628
Closed

Add missing mdu keywords to FMModel class #571

veenstrajelmer opened this issue Oct 23, 2023 · 5 comments · Fixed by #628
Labels
priority: high type: compatibility Changes needed to be compatible with the computational core

Comments

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Oct 23, 2023

Is your feature request related to a problem? Please describe.
Several mdu keywords are missing in the hydrolib core FMModel class. When reading a mdu file that is copied from the FM dia file, a lot of keywords are dropped when reading them. This should not happen. It is also not allowed to add these keywords to the FMModel, while at least part of them are documented (like wrimap_windstress)

MWE with attached mdu file:

import hydrolib.core.dflowfm as hcdfm
file_mdu = r"c:\Users\veenstra\Downloads\mdu_fromdia.mdu"
mdu = hcdfm.FMModel(file_mdu)
mdu.save(file_mdu.replace(".mdu","ignoreextra_hcdfm.mdu"))
# mdu.output.wrimap_windstress

The allowextra file was obtained by overwriting the hcdfm.ini.models.INIBasedModel().Config.extra parameter with Extra.allow. A diff between the ignoreextra and allowextra files show the mdu keywords missing in hydrolib-core.

Describe the solution you'd like
Inclusion of these keywords.

mdu_fromdia.mdu.txt

mdu_fromdia_hcdfm_allowextra.mdu.txt

mdu_fromdia_hcdfm_ignoreextra.mdu.txt

@priscavdsluis priscavdsluis added type: compatibility Changes needed to be compatible with the computational core priority: high labels Apr 9, 2024
@rhutten rhutten added this to To do in HYDROLIB-core via automation Apr 10, 2024
@tim-vd-aardweg tim-vd-aardweg moved this from To do to In progress in HYDROLIB-core Apr 16, 2024
@tim-vd-aardweg
Copy link
Contributor

tim-vd-aardweg commented Apr 16, 2024

I have performed a diff. The diff shows shows that there are around 200 keywords that are in the allowextra file that are not in the ignoreextra file. See Keywords diff.txt.
In this file I have marked keywords that are not in the manual with a *. Keywords that are research keywords according to the manual are marked with **. Finally, some keywords are marked with a ***. In most cases they are mentioned in the manual somewhere, but not in the MDU appendix.

We will start by adding those keywords that are missing in HYDROLIB-core and are mentioned in the Appendix A of the manual.

After that, we have to discuss with @rhutten and @veenstrajelmer what to do with those other keywords.

@tim-vd-aardweg
Copy link
Contributor

tim-vd-aardweg commented Apr 19, 2024

The following keywords have been added:

Section Keyword Type Default value
geometry gridenclosurefile Optional[PolyFile] None
geometry allowbndatbifurcation bool False
geometry slotw2d float 0.001
geometry slotw1d float 0.001
geometry uniformheight1droofgutterpipes float 0.1
geometry dxmin1d float 0.001
geometry uniformtyp1dstreetinlets int -2
geometry stretchtype int 1
geometry zlaybot float -999.0
geometry uniformheight1d float 3.0
geometry zlaytop float -999.0
geometry roofsfile Optional[PolyFile] None
geometry gulliesfile Optional[PolyFile] None
geometry uniformwidth1dstreetinlets float 0.2
geometry uniformheight1dstreetinlets float 0.1
geometry uniformtyp1droofgutterpipes int -2
geometry uniformwidth1droofgutterpipes float 0.1
numerics fixedweirrelaxationcoef float 0.6
numerics implicitdiffusion2d bool False
numerics vertadvtyptem int 6
numerics velmagnwarn float 0.0
numerics transportautotimestepdiff int 0
numerics sethorizontalbobsfor1d2d bool False
numerics diagnostictransport bool False
numerics vertadvtypsal int 6
numerics zerozbndinflowadvection int 0
numerics pure1d int 0
numerics testdryingflooding int 0
numerics logsolverconvergence bool False
numerics fixedweirscheme1d2d int 0
numerics horizontalmomentumfilter bool False
numerics maxnonlineariterations int 100
numerics maxvelocity float 0.0
numerics waterlevelwarn float 0.0
numerics tspinupturblogprof float 0.0
numerics fixedweirtopfrictcoef Optional[float] None
numerics fixedweir1d2d_dx float 50.0
numerics junction1d int 0
numerics fixedweirtopwidth float 3.0
numerics vertadvtypmom int 6
numerics checkerboardmonitor bool False
numerics velocitywarn float 0.0
numerics adveccorrection1d2d int 0
numerics fixedweirtalud float 4.0
wind stresstowind float 0.0
output wrimap_every_dt bool False
output wrimap_input_roughness bool False
output wrimap_flowarea_au bool False
output wrihis_airdensity bool False
output wrimap_flow_flux_q1_main bool False
output wrimap_windstress bool False
output wrishp_genstruc bool False
output wrimap_qin bool False
output wrimap_dtcell bool False
output wrimap_velocity_vectorq bool False
output wrimap_bnd bool False
output wrishp_dambreak bool False
output wrimap_waterdepth_hu bool False
output ncmapdataprecision Literal['single', 'double'] 'double'
output nchisdataprecision Literal['single', 'double'] 'double'
output wrimap_interception bool False
output wrimap_airdensity bool False
output wrimap_volume1 bool False
output wrimap_ancillary_variables bool False
output wrimap_chezy_on_flow_links bool False

@tim-vd-aardweg tim-vd-aardweg moved this from In progress to Ready to review in HYDROLIB-core Apr 22, 2024
@tim-vd-aardweg tim-vd-aardweg linked a pull request Apr 22, 2024 that will close this issue
@MRVermeulenDeltares MRVermeulenDeltares moved this from Ready to review to Review in progress in HYDROLIB-core Apr 22, 2024
@MRVermeulenDeltares MRVermeulenDeltares moved this from Review in progress to Test in HYDROLIB-core Apr 22, 2024
@tim-vd-aardweg
Copy link
Contributor

@veenstrajelmer Do you maybe have a short moment to test this issue before we merge it into main? In this issue we have added the keywords listed above (the ones that are mentioned in the manual). In a separate issue we will add support for other/unknown keywords/sections.

@rhutten
Copy link
Collaborator

rhutten commented Apr 29, 2024

After an extended discussion is decided by the PO to only add keywords which are included in manual.
Other keywords will not be added with a validation to HYDROLIB-core. However, we will make an option to read and write this keywords. Note that the modeller cannot change these keywords.

@tim-vd-aardweg
Copy link
Contributor

I tested the issue by creating a very simply model in the latest 1D2D GUI (2024.03 release candidate) and running it. I then took the .mdu file from the .dia file and tried importing that in HYDROLIB-core. That all went fine. I checked a couple (not all) of the keywords above and they could be set and were correctly saved.

I will therefore merge this issue back into main.

tim-vd-aardweg added a commit that referenced this issue May 2, 2024
HYDROLIB-core automation moved this from Test to Done May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high type: compatibility Changes needed to be compatible with the computational core
Projects
HYDROLIB-core
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants