-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: external forcings converter #647
Draft
arthurvd
wants to merge
17
commits into
main
Choose a base branch
from
feature/621_extforce_converter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
559ddac
Initial setup of ext_old_to_new tool
arthurvd 1bb5532
converter can now convert e02_dflowfm\f006_external_forcing\c011_extr…
FlorisBuwaldaDeltares 2ab564f
modified tests to no longer use local directories
FlorisBuwaldaDeltares 1fb7583
added extrapolate_slr case
FlorisBuwaldaDeltares 851975d
Added some exception handling, now validate Meteo quantity before mak…
FlorisBuwaldaDeltares f8dfbce
added basinsquares test, fixed rainschematic.ext as an uncommented li…
FlorisBuwaldaDeltares 34704fa
restore rainschematic.ext as otherwise unit tests get broken
FlorisBuwaldaDeltares c1033df
autoformat: isort & black
FlorisBuwaldaDeltares fdb2981
chore!: Refactored ext old converter to prepare for more other quanti…
arthurvd 096d194
Merge branch 'main' into feature/621_extforce_converter
arthurvd 9266e7b
Remove non-existing structure file reference in a test MDU
arthurvd 314fbc8
Remove obsolete keywords from example windcase.mdu
arthurvd 9dd1d70
New feature `exclude_unset` in ModelSaveSettings
arthurvd e2720ad
Combine power of LegacyFMModel and ResearchFMModel
arthurvd 26e6d9b
Work-in-progress making converter tool more flexible for repeated runs
arthurvd 2fb8c8b
Merge branch 'main' into feature/621_extforce_converter
arthurvd 037cbc3
feat: initial condition converter from old external forcing file
MAfarrag File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
AstronomicCorrection, Constant, T3D. | ||
|
||
""" | ||
|
||
import logging | ||
import re | ||
from pathlib import Path | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
from typing import Literal | ||
|
||
from pydantic import Field | ||
|
||
from .models import FMModel, General | ||
|
||
|
||
class LegacyGeneralAsModel(General): | ||
""" | ||
The `[Model]` section in a legacy MDU file. | ||
|
||
This a small helper class to support legacy versions of .mdu files, where | ||
the `[General]` section with metadata did not exist yet, and was called | ||
`[Model]` instead. | ||
""" | ||
|
||
_header: Literal["Model"] = "Model" | ||
mduformatversion: str = Field("1.06", alias="MDUFormatVersion") | ||
|
||
|
||
class LegacyFMModel(FMModel): | ||
""" | ||
Legacy version of an MDU file, specifically for: MDUFormatVersion <= 1.06. | ||
|
||
This a small wrapper class to support legacy versions of .mdu files, where | ||
the `[General]` section with metadata did not exist yet, and was called | ||
`[Model]` instead. | ||
""" | ||
|
||
model: LegacyGeneralAsModel = Field(default_factory=LegacyGeneralAsModel) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@priscavdsluis / @tim-vd-aardweg : what do you guys think about this extra "legacy.py" inside the MDU directory?
As long as we don't have versioned support for several old/newer formats of some of our filetypes, this was the solution I came up with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arthurvd / @priscavdsluis
I am not entirely sure to be honest. My first question is: Do we want tools in HYDROLIB-core, or do we create a separate package for tools that use HYDROLIB-core?
But let's assume we do want to add tools to the HYDROLIB-core package. I think my next question is: Should we implement this 'legacy' model into hydrolib, or should it be defined in the tool that uses it? Also, the change you request touches on a broader topic that should be discussed: How do we handle old/deprecated files/sections/keywords. Your requested change is due to the fact that we cannot handle older/unexpected file formats. And your change fixes things for your specific use case.
Having said all that, my thoughts are as follows:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @arthurvd / @tim-vd-aardweg / @priscavdsluis , I know you didn't ask my opinion but I still want to ask some questions regarding this.
Previously I remember a presentation was given with a picture showing hydrolib-core and hydrolib.
And how it should be used by tools.
See https://www.deltares.nl/expertise/projecten/hydrolib-gezamenlijke-automatisering-op-watersysteemmodellen
Should the converter not be a tool build on top of hydrolib-core?
Especially if the support for the old file format is dropped and no longer backwards compatible supported by the kernels.
Should hydrolib-core still support the old format, or only the new format from version x?