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

Feat/316 forcing vectorquantities #381

Merged
merged 9 commits into from
Oct 25, 2022

Conversation

arthurvd
Copy link
Member

Fixes #316.

arthurvd and others added 4 commits October 19, 2022 11:34
…ndaries

* VectorQuantityUnitPairs object represents both the "vector=" headerline and *all* coupled vector elements's QuantityUnitPairs below.
* `ScalarOrVectorQUP = Union[QuantityUnitPair, VectorQuantityUnitPairs]` for convenience.
* same approach taken as was already done for the original QuantityUnitPair creation and the vertPositionIndex modification, namely via a root_validator (pre=True) in both TimeSeries and T3D.
* the actual root_validator `_validate_vectordefinition_and_update_quantityunitpairs()` has been placed in `class ForcingBase` to share all code.
* unit tests added, both reading from file and constructing in memory.
hydrolib/core/io/bc/models.py Show resolved Hide resolved
hydrolib/core/io/bc/models.py Show resolved Hide resolved
hydrolib/core/io/bc/models.py Outdated Show resolved Hide resolved
hydrolib/core/io/bc/models.py Outdated Show resolved Hide resolved
# created VectorQUPs.
quantityunitpairs_with_vectors = []

# If one quantity is "time", it must be the first one.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the first quantity always be "time"? So shouldn't we raise an error if the first quantity is not "time"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not required by D-HYDRO. We only use it in hydrolib-core as the only pragmatic way to detect where the vector elements start.

hydrolib/core/io/bc/models.py Show resolved Hide resolved
* VectorQuantityUnitPairs now has Config, validate_assignment = True to re-run validators when its children quantityunitpair list gets re-assigned.
* Some extra vector validation is now directly in VectorQuantityUnitPairs, when creating these objects from scratch (as opposed to reading from file)
* the original vector validator that used to be in ForcingBase was now moved to dedicated class VectorQUPValidation, used by both TimeSeries and T3D.
* This new class VectorQUPValidation also has separate subroutine for validating number of vector elements, again useful when constructing TimeSeries or T3D from scratch (as opposed to reading from file)
* some extra unit tests asserting errors when quantity names are wrong or wrong amount.
* test_bc, cleanup: time unit "m" -> parameter.
@sonarcloud
Copy link

sonarcloud bot commented Oct 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.4% 0.4% Duplication

Copy link
Contributor

@tim-vd-aardweg tim-vd-aardweg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review OK! As discussed, I will create a separate issue for a refactoring of vector implementation.

@tim-vd-aardweg tim-vd-aardweg merged commit 28e4243 into main Oct 25, 2022
@tim-vd-aardweg tim-vd-aardweg deleted the feat/316_forcing_vectorquantities branch October 25, 2022 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ForcingModel support for uxuy (or supply separately to dflowfm kernel)
2 participants