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

Introduce BenchmarkTimeVaryingMetric for non-map metrics that vary over time #3184

Closed
wants to merge 2 commits into from

Conversation

esantorella
Copy link
Contributor

Summary:
Context: There are situations where a metric is not a MapMetric, but its value changes depending on time.

This diff:
Introduces BenchmarkTimeVaryingMetric, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Differential Revision: D67224949

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Dec 16, 2024
Copy link

vercel bot commented Dec 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ax-archive ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2024 3:57pm

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67224949

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 99.25926% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.83%. Comparing base (f71703f) to head (0079197).

Files with missing lines Patch % Lines
ax/benchmark/benchmark_metric.py 97.87% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3184      +/-   ##
==========================================
- Coverage   95.83%   95.83%   -0.01%     
==========================================
  Files         505      505              
  Lines       50989    51003      +14     
==========================================
+ Hits        48866    48877      +11     
- Misses       2123     2126       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

esantorella added a commit to esantorella/Ax that referenced this pull request Dec 17, 2024
…er time (facebook#3184)

Summary:

**Context**: There are situations where a metric is not a MapMetric, but its value changes depending on time.

**This diff**:
Introduces `BenchmarkTimeVaryingMetric`, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Differential Revision: D67224949
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67224949

esantorella added a commit to esantorella/Ax that referenced this pull request Dec 18, 2024
…er time (facebook#3184)

Summary:

**Context**: There are situations where a metric is not a MapMetric, but its value changes depending on time.

**This diff**:
Introduces `BenchmarkTimeVaryingMetric`, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Reviewed By: Balandat

Differential Revision: D67224949
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67224949

esantorella added a commit to esantorella/Ax that referenced this pull request Dec 18, 2024
…er time (facebook#3184)

Summary:

**Context**: There are situations where a metric is not a MapMetric, but its value changes depending on time.

**This diff**:
Introduces `BenchmarkTimeVaryingMetric`, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Reviewed By: Balandat

Differential Revision: D67224949
esantorella added a commit to esantorella/Ax that referenced this pull request Dec 18, 2024
…er time (facebook#3184)

Summary:

**Context**: There are situations where a metric is not a MapMetric, but its value changes depending on time.

**This diff**:
Introduces `BenchmarkTimeVaryingMetric`, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Reviewed By: Balandat

Differential Revision: D67224949
Summary:

**Context**: Currently, benchmark metric tests conflate whether the metadata consumed by `fetch_trial_data` has multiple time steps with whether the output data should be `MapData`, and don't explicitly address the separate cases of whether or not there is a `BackendSimulator` attached to the metadata. They are also not organized well. These issues will become more pressing when we start to allow `BenchmarkMetric` to consume data with multiple time steps and/or use a backend simulator

**This diff**:
* Organizes those unit tests better
* Adds some more checks

Reviewed By: Balandat

Differential Revision: D67281379
…er time (facebook#3184)

Summary:

**Context**: There are situations where a metric is not a MapMetric, but its value changes depending on time.

**This diff**:
Introduces `BenchmarkTimeVaryingMetric`, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Reviewed By: Balandat

Differential Revision: D67224949
esantorella added a commit to esantorella/Ax that referenced this pull request Dec 19, 2024
…er time (facebook#3184)

Summary:

**Context**: There are situations where a metric is not a MapMetric, but its value changes depending on time.

**This diff**:
Introduces `BenchmarkTimeVaryingMetric`, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Reviewed By: Balandat

Differential Revision: D67224949
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67224949

esantorella added a commit to esantorella/Ax that referenced this pull request Dec 19, 2024
…er time (facebook#3184)

Summary:

**Context**: There are situations where a metric is not a MapMetric, but its value changes depending on time.

**This diff**:
Introduces `BenchmarkTimeVaryingMetric`, which is not a MapMetric, can consume data with multiple time steps, is available while running, and requires a backend simulator.

Reviewed By: Balandat

Differential Revision: D67224949
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 9f57bb2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants