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

Fix resolution for simulation and inflow calc #461

Merged
merged 1 commit into from
May 6, 2024

Conversation

erny-powel
Copy link
Collaborator

When implementing run_simulation and run_inflow_calculation I used our predefined protobuf time series resolutions for simulation and inflow calculation resolution. This is not correct and those might run on resolutions we don't allow for time series.

Update proto to use
https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/duration.proto, which is used by the Mesh server starting with v2.14.0.

See https://github.com/Volue/energy-mesh/issues/5305 (internal).
See https://github.com/Volue/energy-mesh/pull/5317 (internal).

@erny-powel erny-powel added this to the Version 1.8 milestone May 2, 2024
@erny-powel erny-powel self-assigned this May 2, 2024
@erny-powel erny-powel force-pushed the erny/hydsim-resolution-timedelta branch from 94beeaa to 105c9e8 Compare May 2, 2024 10:21
When implementing run_simulation and run_inflow_calculation I used our
predefined protobuf time series resolutions for simulation and inflow
calculation resolution. This is not correct and those might run on
resolutions we don't allow for time series.

Update proto to use
https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/duration.proto,
which is used by the Mesh server starting with v2.14.0.

See Volue/energy-mesh#5305 (internal).
See Volue/energy-mesh#5317 (internal).
@erny-powel erny-powel force-pushed the erny/hydsim-resolution-timedelta branch from 105c9e8 to 17f78c2 Compare May 6, 2024 07:43
@erny-powel erny-powel marked this pull request as ready for review May 6, 2024 07:44
@erny-powel erny-powel requested review from tnoczyns-volue and simia May 6, 2024 07:44
@erny-powel erny-powel merged commit 12d7c7e into master May 6, 2024
9 checks passed
@erny-powel erny-powel deleted the erny/hydsim-resolution-timedelta branch May 6, 2024 07:59
@@ -15,7 +15,9 @@ Compatible with
New features
~~~~~~~~~~~~~~~~~~

- TBA
- It's now possible to specify the resolution of a hydro simulation or inflow
calculation using the optinal `resolution` argument to `run_simulation` and
Copy link
Collaborator

Choose a reason for hiding this comment

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

optinal -> optional ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good catch 😄

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.

3 participants