DHI / mikeio

Read, write and manipulate dfs0, dfs1, dfs2, dfs3, dfsu and mesh files.
https://dhi.github.io/mikeio
BSD 3-Clause "New" or "Revised" License
136 stars 53 forks source link

Bug in writting of non-equidistant dfs2-files #595

Closed sweco-sejurj closed 8 months ago

sweco-sejurj commented 10 months ago

Describe the bug When writing a time-non-equidistant dfs2 file. This might be incorrectly interpreted by MikeZero.

I have been trying to create a non-equidistant 2D precipitation file. Although the file seems fine at first glance in MikeZero, upon selecting this as precipitation-file in Mike+ (2023) the timesteps are reset to equidistant timesteps. Also in MikeZero the timesteps will be wrong from that point.

To Reproduce Run the test test_write_non_equidistant_data() in tests/test_dfs2.py Open the produced file in Mike Zero. Observe that the timesteps below the map-data are identical to Python.

Open the Time Steps (Edit --> Time steps) Observe that these timesteps are different than the ones indicated in Python. These seem to be the ones that are actually used.

Expected behavior Non-equidistant timesteps that are actually used in the Mike model runs.

Screenshots image

System information:

Thanks again for a great IO!

ecomodeller commented 10 months ago

I think the problem with non-equidistant time dfs2 files are that they might be valid dfs files (i.e. in the sense that they can be written with mikecore), but they are not correctly interpreted by any of the editors. @JesperGr should we remove the possibility of writing non-equidistant dfs2 files from MIKE IO?

jsmariegaard commented 10 months ago

There can be good reasons for writing dfs2 files with non-equidistant time axis - e.g. satellite images. As far as I know, the engines would also be able to interpret those correctly (I wonder if this is true for all engines 🤔 ), but it seems from the above screenshots that the viewer is not capable.

Hendrik1987 commented 10 months ago

I had the same issue with dfsu #588.

JesperGr commented 10 months ago

Most engines are capable of handling a dfs2/dfsu file with a non-equidistant time axis, and it is sometimes very usefull to do so. So, I would probably not remove the possibility for creating those - but at least make sure that if the time is equidistantly distributed, then also the equidistant time axis is written to the dfs file.

Then we may look into updating the MIKE Zero tooling to support the non-equdistant time axis also for dfs2/dfsu.

ecomodeller commented 10 months ago

@sweco-sejurj are you satisfied with the answers, that the bug doesn't really belong in MIKE IO, but rather in MIKE Zero?

sweco-sejurj commented 10 months ago

(Sorry for the late response, I was on holiday)

Thanks for these insights! My actual use case was the use of Spatially variable rain in Mike+ 2023 (so Mike21). Apparently this feature does not handle the non-equidistant time axis properly. Just the process of selecting the non-equidistant file as 2D precipitation file, will already 'reset' the time axis to an equidistant time axis.

I would suggest to add a warning to the documentation and/or a python warning when writing this type of file, as the Mike GUI's do not give proper warnings/errors that the functionality is not (always) supported.

JesperGr commented 10 months ago

@sweco-sejurj Is MIKE+ actually reseting the time axis of the file on the disc, i.e. changing the file on the disc? Or is it just reporting incorrectly the time axis? All the engines are (should be) supporting the non-equidistant time axis, so if MIKE+ is just reporting incorrectly, the engines should just take the non-equidistant input and run.

sweco-sejurj commented 10 months ago

From what I recall, Mike+ is actually already changing the file on disk after merely selecting the file from the file browser. I will verify this behaviour later.

Also from my check of the volume balance in the logging, it is using the 'file with time-axis reset' in the calculation. (But this behaviour of the engine is possible the result of the earlier change by the GUI)

But I will verify this again on short notice and get back to this discussion.

sweco-sejurj commented 8 months ago

I cannot reproduce the error currently. We must have had some workflow that actually changed the file, but for now I am closing this issue.