-
Notifications
You must be signed in to change notification settings - Fork 1
Add support for multidimensional metrics #844
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
Add support for multidimensional metrics #844
Conversation
wk9874
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a full review since I'm encountering the bug which I've left as a comment, so haven't yet tested if it works correctly
simvue/api/objects/grids.py
Outdated
| mapping from offline identifier to new online identifier. | ||
| """ | ||
| for run_id in self._staging.pop("runs", []): | ||
| self.attach_to_run(run_id=id_mapping[run_id]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the run id is not found in the mapping?
(should never happen, but I think that is the cause of the offline metrics issue which is plaguing us)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added additional raise statement
|
|
||
| @pydantic.validate_call | ||
| def get_run_values(self, step: int) -> dict: | ||
| """Retrieve values for this grid from the server for a given run at a given step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this function do? Are the values the ticks? In which case why do we need step as my understanding is the grid axes should be fixed throughout right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns the values at a given step from a grid for a given run and metric, missing parameters and wrong so modified now
|
|
||
| @pydantic.validate_call | ||
| def get_run_span(self, run_id: str) -> dict: | ||
| """Retrieve span for this grid from the server for a given run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also what is the span?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alahiff I think you would be able to explain this better
simvue/sender.py
Outdated
| "folders", | ||
| "tags", | ||
| "alerts", | ||
| "grids", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should grids be created before runs? Wont they require a run ID to attach to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No Grid.new does not require a run, the attachment happens later, and can only happen if a run is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in offline mode, attach_to_run is called inside on_reconnect, which will be called when the sender tries to send the grid. Meaning if the cache contains a run and a grid, it will try to connect the grid to a run which does not yet exist, since the run has not yet been sent
|
@kzscisoft does the client work for getting 2D metrics? |
Fixes offline mode for multi dimensional metrics
wk9874
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Please open a new PR on the docs repo to compliment this. In particular, please mention that the shape of 2D metrics may be unexpected - axes are expected in (x, y) format, while data is in (row, column) format.
Add Support for Tensor Metrics
Issue: #833
Python Version(s) Tested: 3.13
Operating System(s): Ubuntu 25.04
Documentation PR: https://github.com/simvue-io/docs/pull/77
📝 Summary
Allows the user to define a grid and then log tensors as metrics (or multidimensional metrics) to that grid.
🔄 Changes
Low Level API
Gridclass for defining a new grid:GridMetricsclass for logging metrics across multiple grids for a given run:User API
For the user,
log_metricshas been updated to allow numpy arrays as metric values, the user must firstly assign or create a grid for that metric:Note if the user does not specify
axes_ticksandaxes_labelsit is assumed the grid already exists.✔️ Checklist