#369: Support the old style external forcings file #471

priscavdsluis · 2023-03-17T10:11:24Z

Fixes #369

…category.

arthurvd

Hi @priscavdsluis : your new additions of yesterday look good.
I did an overall review, please see my suggestions for some minor changes here and there.

arthurvd · 2023-04-13T06:53:02Z

hydrolib/core/dflowfm/extold/models.py

@@ -425,66 +455,73 @@ def validate_operand(cls, value):

    @root_validator(skip_on_failure=True)
    def validate_forcing(cls, values):
-        def alias(field_key: str):
-            return cls.__fields__[field_key].alias
+        class Field:


Somewhat confusing name, because this is not the pydantic Field, maybe rename to _Field, or LocalField or CheckField?

arthurvd · 2023-04-13T07:00:21Z

hydrolib/core/dflowfm/extold/models.py

+            raise ValueError(error)
+
+        only_allowed_when(ifrctype, quantity, ExtOldQuantity.FrictionCoefficient)
+        only_allowed_when(averagingtype, method, 6)


Suggested change

only_allowed_when(averagingtype, method, 6)

only_allowed_when(averagingtype, method, ExtOldMethod.Averaging)

Done for each check

arthurvd · 2023-04-13T07:00:33Z

hydrolib/core/dflowfm/extold/models.py

+
+        only_allowed_when(ifrctype, quantity, ExtOldQuantity.FrictionCoefficient)
+        only_allowed_when(averagingtype, method, 6)
+        only_allowed_when(relativesearchcellsize, method, 6)


Suggested change

only_allowed_when(relativesearchcellsize, method, 6)

only_allowed_when(relativesearchcellsize, method, ExtOldMethod.Averaging)

arthurvd · 2023-04-13T07:00:57Z

hydrolib/core/dflowfm/extold/models.py

+        only_allowed_when(ifrctype, quantity, ExtOldQuantity.FrictionCoefficient)
+        only_allowed_when(averagingtype, method, 6)
+        only_allowed_when(relativesearchcellsize, method, 6)
+        only_allowed_when(extrapoltol, method, 5)


Suggested change

only_allowed_when(extrapoltol, method, 5)

only_allowed_when(extrapoltol, method, ExtOldMethod.InterpolateTime)

arthurvd · 2023-04-13T07:01:10Z

hydrolib/core/dflowfm/extold/models.py

+        only_allowed_when(averagingtype, method, 6)
+        only_allowed_when(relativesearchcellsize, method, 6)
+        only_allowed_when(extrapoltol, method, 5)
+        only_allowed_when(percentileminmax, method, 6)


Suggested change

only_allowed_when(percentileminmax, method, 6)

only_allowed_when(percentileminmax, method, ExtOldMethod.Averaging)

hydrolib/core/dflowfm/extold/models.py

Co-authored-by: Arthur van Dam <arthurvd@gmail.com>

…ltares/HYDROLIB-core into feat/369-support-old-ext-file

veenstrajelmer · 2023-04-13T09:43:36Z

"""
import hydrolib.core.dflowfm as hcdfm
hcdfm.ExtOldForcing(quantity='initialsalinity',
filename='test.nc',
varname='so',
filetype=hcdfm.ExtOldFileType.NetCDFGridData,
method=hcdfm.ExtOldMethod.InterpolateTimeAndSpaceSaveWeights, #3
operand=hcdfm.Operand.override, #O
)
"""
Results in "AttributeError: Averaging"

…ng in source code.

MRVermeulenDeltares · 2023-04-13T11:55:27Z

hydrolib/core/dflowfm/extold/models.py

-    SpatiallyVaryingWeather = 3
-    """3. Spatially varying weather"""
+    SpatiallyVaryingWindPressure = 3
+    """3. Spatially varying wind and pressure"""


is there a difference between windpressure and wind & pressure?

is there a difference between windpressure and wind & pressure?

I don't know what windpressure is. This file type here contains three fields: wind (with two vector components) and one airpressure.

veenstrajelmer · 2023-04-13T12:04:01Z

@priscavdsluis: Testing current branch status results in correct old extfile with several meteo parameters (airpressure, wind, charnock, solarradiation) and initial conditions (salinity and temperature). So successful test.

hydrolib/core/dflowfm/extold/__init__.py

tim-vd-aardweg · 2023-04-13T12:22:43Z

hydrolib/core/dflowfm/extold/models.py

 from hydrolib.core.dflowfm.extold.parser import Parser
 from hydrolib.core.dflowfm.extold.serializer import Serializer
 from hydrolib.core.dflowfm.polyfile.models import PolyFile
 from hydrolib.core.dflowfm.tim.models import TimModel

-
-class TracerQuantity(str, Enum):
+HEADER = """


Did you copy paste this from somewhere? 😄
Do you want me to review this header as well? Or, since it is just a header, let the users report any issues with it (if they find any).

Yes this one was provided by @veenstrajelmer, but we will probably update it to a more recent one or change it such that it conforms to our own implementation.

hydrolib/core/dflowfm/extold/models.py

veenstrajelmer · 2023-04-13T12:34:10Z

@priscavdsluis: Testing current branch status results in correct old extfile with several meteo parameters (airpressure, wind, charnock, solarradiation) and initial conditions (salinity and temperature). So successful test.

Tested again and still works successfully

tim-vd-aardweg · 2023-04-13T13:37:08Z

hydrolib/core/dflowfm/extold/models.py

+                sourcemask, filetype, valid_dependency_value="4 or 6"
+            )
+
+        if (


Let's have a very short discussion if this works as intented. It always confuses me 😊

Nevermind, this should work! 😊

veenstrajelmer · 2023-04-14T05:51:10Z

@priscavdsluis After a conversation yesterday, I discovered the quantity nudge_salinity_temperature is not supported. It is not documented and also not in the header I provided, but is mentioned in the user manual in a footnote. Could it be added? I will search for an updated header also.

…ltares/HYDROLIB-core into feat/369-support-old-ext-file

sonarqubecloud · 2023-04-14T07:42:46Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
11 Code Smells

No Coverage information
0.0% Duplication

veenstrajelmer · 2023-04-14T09:31:06Z

@priscavdsluis @tim-vd-aardweg Tested and all works now

priscavdsluis and others added 30 commits March 17, 2023 11:10

#369: Add initial setup ExtForcing

4f1937d

autoformat: isort & black

4976309

#369: Add FileType to use for the ExtForcing.filetype attribute

793d570

#369: Add Method enum to use for the ExtForcing.method attribute

fd65a38

#369: Add Operand enum to use for the ExtForcing.operand attribute

7a28e5d

autoformat: isort & black

f37231c

#369: Add extra keywords and fix aliases

c197862

Merge branch 'main' into feat/369-support-old-ext-file

947b024

autoformat: isort & black

db4344c

#369: Add the external forcings for the boundary conditions category.

7c2cf0b

autoformat: isort & black

8436d76

#369: Change enums to upper camel case

f04492a

#369: Add the external forcings for the meteo category.

1caa607

autoformat: isort & black

87ea229

#369: Add the external forcings for the structure category.

6ea2635

#369: Add the external forcings for the initial fields category.

494f98c

autoformat: isort & black

2b81ac8

#369: Add the external forcings for the spatially varying properties …

2ea89cc

…category.

#369: Add the external forcings for the miscellaneous category.

4116553

autoformat: isort & black

02b5bcc

#369: Move all categories into one

c9ef619

autoformat: isort & black

7d365d8

#369: Add more quantities that are not listed in manual

6cc3178

#369: Use the quantity class

cb63f5b

#369: Use correct import

31c802b

autoformat: isort & black

2723d4a

#369: Update quantity validation and add tests

38c6172

#369: Update test names

5907f55

autoformat: isort & black

d70dd39

#369: Make quantity validation case-insensitive

de372d2

arthurvd requested changes Apr 13, 2023

View reviewed changes

priscavdsluis and others added 6 commits April 13, 2023 09:33

Merge branch 'main' into feat/369-support-old-ext-file

f86e6f6

#369: Rename local class Field to _Field

73924ad

#369: Replace integers with enum definitions.

745505d

#369: Improve tests

532b1be

#369: Update docstrings

66d1130

Co-authored-by: Arthur van Dam <arthurvd@gmail.com>

Merge branch 'feat/369-support-old-ext-file' of https://github.com/De…

ac8c016

…ltares/HYDROLIB-core into feat/369-support-old-ext-file

priscavdsluis and others added 3 commits April 13, 2023 13:14

Merge branch 'main' into feat/369-support-old-ext-file

1707c37

#369: Use ExtOldMethod.AveragingSpace instead of ExtOldMethod.Averagi…

c6d04b1

…ng in source code.

autoformat: isort & black

4c7f342

MRVermeulenDeltares reviewed Apr 13, 2023

View reviewed changes

tim-vd-aardweg reviewed Apr 13, 2023

View reviewed changes

hydrolib/core/dflowfm/extold/__init__.py Show resolved Hide resolved

tim-vd-aardweg reviewed Apr 13, 2023

View reviewed changes

hydrolib/core/dflowfm/extold/models.py Show resolved Hide resolved

tim-vd-aardweg reviewed Apr 13, 2023

View reviewed changes

priscavdsluis and others added 7 commits April 14, 2023 09:00

#369: Add ExtOldExtrapolationMethod to __init__.py

a5c2fde

#369: Update docs

7425aea

#369: Add "nudge_salinity_temperature" to ext quantities

5f92148

autoformat: isort & black

e229c13

#369: Improve docs

28f827d

Merge branch 'feat/369-support-old-ext-file' of https://github.com/De…

65f4938

…ltares/HYDROLIB-core into feat/369-support-old-ext-file

Merge branch 'main' into feat/369-support-old-ext-file

3a88511

priscavdsluis merged commit cbb80af into main Apr 14, 2023

priscavdsluis deleted the feat/369-support-old-ext-file branch April 14, 2023 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#369: Support the old style external forcings file #471

#369: Support the old style external forcings file #471

priscavdsluis commented Mar 17, 2023 •

edited

Loading

arthurvd left a comment

arthurvd Apr 13, 2023

priscavdsluis Apr 13, 2023

arthurvd Apr 13, 2023

priscavdsluis Apr 13, 2023

arthurvd Apr 13, 2023

arthurvd Apr 13, 2023

arthurvd Apr 13, 2023

veenstrajelmer commented Apr 13, 2023

MRVermeulenDeltares Apr 13, 2023

arthurvd Apr 13, 2023

veenstrajelmer commented Apr 13, 2023 •

edited

Loading

tim-vd-aardweg Apr 13, 2023

priscavdsluis Apr 14, 2023

veenstrajelmer commented Apr 13, 2023 •

edited

Loading

tim-vd-aardweg Apr 13, 2023

tim-vd-aardweg Apr 14, 2023

veenstrajelmer commented Apr 14, 2023 •

edited

Loading

sonarqubecloud bot commented Apr 14, 2023

veenstrajelmer commented Apr 14, 2023

	only_allowed_when(averagingtype, method, 6)
	only_allowed_when(averagingtype, method, ExtOldMethod.Averaging)

	only_allowed_when(relativesearchcellsize, method, 6)
	only_allowed_when(relativesearchcellsize, method, ExtOldMethod.Averaging)

	only_allowed_when(extrapoltol, method, 5)
	only_allowed_when(extrapoltol, method, ExtOldMethod.InterpolateTime)

	only_allowed_when(percentileminmax, method, 6)
	only_allowed_when(percentileminmax, method, ExtOldMethod.Averaging)

#369: Support the old style external forcings file #471

#369: Support the old style external forcings file #471

Conversation

priscavdsluis commented Mar 17, 2023 • edited Loading

arthurvd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veenstrajelmer commented Apr 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veenstrajelmer commented Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veenstrajelmer commented Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veenstrajelmer commented Apr 14, 2023 • edited Loading

sonarqubecloud bot commented Apr 14, 2023

veenstrajelmer commented Apr 14, 2023

priscavdsluis commented Mar 17, 2023 •

edited

Loading

veenstrajelmer commented Apr 13, 2023 •

edited

Loading

veenstrajelmer commented Apr 13, 2023 •

edited

Loading

veenstrajelmer commented Apr 14, 2023 •

edited

Loading