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

[SDK] Add Some Checks for metrics Field in report_metrics() Interface #2421

Closed
Electronic-Waste opened this issue Sep 4, 2024 · 4 comments
Closed
Assignees
Labels
area/sdk good first issue Good for newcomers help wanted Extra attention is needed kind/feature

Comments

@Electronic-Waste
Copy link
Member

What you would like to be added?

We decide to add some conditional checks in report_metrics() to ensure it works as expected when encounting some tricky corner case and enhance its robustness.

Why is this needed?

In the AutoML and Training WG Community Call, @tenzen-y asked what would happen if we pushed the same metrics twice in a single container. However, the current version of the report_metrics() API does not process this corner case.

Thus, we decided to raise an issue discussing the possible corner case that report_metrics() might meet with. Everyone is welcome to put your valuable insights and suggestions here!

Love this feature?

Give it a 👍 We prioritize the features with most 👍

@andreyvelich
Copy link
Member

/remove-label lifecycle/needs-triage
/area sdk
/good-first-issue

Copy link

@andreyvelich:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/remove-label lifecycle/needs-triage
/area sdk
/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-prow google-oss-prow bot added area/sdk good first issue Good for newcomers help wanted Extra attention is needed and removed lifecycle/needs-triage labels Sep 4, 2024
@Electronic-Waste
Copy link
Member Author

I'd like to work on this since this is related to my GSoC project: #2340

/assign

@Electronic-Waste
Copy link
Member Author

After investigating this issue, I find that:

  1. If we pass two same keys in metrics: the latter one will overwrite the former one.

  2. If we call report_metrics() twice and report the same metrics: they won't interfere with each other since they have different timestamp and GetObservationLog will retrieve both of them to the return value.

rows, err := d.db.Query("SELECT time, metric_name, value FROM observation_logs WHERE trial_name = ?"+qstr+" ORDER BY time",
qfield...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sdk good first issue Good for newcomers help wanted Extra attention is needed kind/feature
Projects
None yet
Development

No branches or pull requests

2 participants