Check quantity units in match() - #716
Open
ecomodeller wants to merge 2 commits into
Open
ecomodeller wants to merge 2 commits into
ecomodeller wants to merge 2 commits into
Conversation
Quantity.is_compatible now compares units only, as written. Names differ
for the same quantity ("Water Level" vs "Surface Elevation"), so they are
not compared. An empty, "undefined" or "Undefined" unit is compatible
with anything.
match() raises when a model result's unit differs from the observation's,
unless check_quantity="ignore".
Quantity.from_mikeio_eum_name stores the unit's short name ("m"), the
same form from_mikeio_iteminfo already uses, so both constructors agree.
Closes #697
Unit strings from CF netCDF (e.g. ERA5 'Degree true') never equal the mikeio short names, so a strict default rejected ordinary netCDF-vs-MIKE comparisons.
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #697.
Impact
Existing
match()calls behave exactly as before. The new unit check is off by default.What changes for users:
New opt-in check in
match().check_quantity="error"raises when a model result's unit differs from the observation's. The default is"ignore": units are compared exactly, and CF unit strings (e.g. ERA5"Degree true") never equal mikeio's ("degree"), so a strict default would reject ordinary netCDF-vs-MIKE comparisons.Quantity.is_compatiblereturns different results. It compares units only, as written; names are ignored. An empty,"undefined"or"Undefined"unit is compatible with anything.Quantity("Water Level", "m")vsQuantity("Surface Elevation", "m")FalseTrueQuantity("Water Level", "m")vsQuantity.undefined()FalseTrueQuantity("Water Level", "m")vsQuantity("Water Level", "meter")FalseFalseQuantity("Undefined", "m")vsQuantity("Discharge", "m^3/s")TrueFalseNames are not compared because the same quantity is named differently on the two sides; a name check rejected 36 matches in the existing test suite, all of them this case. Nothing in modelskill called
is_compatiblebefore this PR.Quantity.from_mikeio_eum_namereturns short unit names, the formfrom_mikeio_iteminfoalready uses:"m"instead of"meter","m^3/s"instead of"meter_pow_3_per_sec". Code that compares against the old strings will need updating.🤖 Generated with Claude Code