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

Temporary Commit at 8/23/2024, 3:24:47 PM #2701

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bernardbeckerman
Copy link
Contributor

Differential Revision: D61730570

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

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

@facebook-github-bot
Copy link
Contributor

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

bernardbeckerman pushed a commit to bernardbeckerman/Ax that referenced this pull request Aug 26, 2024
Summary:
`MapData` has an attribute `df` that takes only the last row from each of `map_df`'s trial-arm-metric groups (when sorted by map_key values), and drops `map_key` columns.

This diff makes it so that `map_key` columns are still present in the `df` attribute, showing their values for each kept row. This will make it easier to understand and model partially complete trials in the future.

[RfC] I've patched up a few tests that assert these dataframes have a specific form, but there don't seem to be any tests failing for functional reasons (since the unused map_key columns would just go unused if not needed). Putting this up as an RfC though in case others feel strongly that these columns should be dropped.

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

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

bernardbeckerman pushed a commit to bernardbeckerman/Ax that referenced this pull request Aug 26, 2024
Summary:
`MapData` has an attribute `df` that takes only the last row from each of `map_df`'s trial-arm-metric groups (when sorted by map_key values), and drops `map_key` columns.

This diff makes it so that `map_key` columns are still present in the `df` attribute, showing their values for each kept row. This will make it easier to understand and model partially complete trials in the future.

[RfC] I've patched up a few tests that assert these dataframes have a specific form, but there don't seem to be any tests failing for functional reasons (since the unused map_key columns would just go unused if not needed). Putting this up as an RfC though in case others feel strongly that these columns should be dropped.

Reviewed By: Balandat

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

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

bernardbeckerman pushed a commit to bernardbeckerman/Ax that referenced this pull request Aug 26, 2024
Summary:
`MapData` has an attribute `df` that takes only the last row from each of `map_df`'s trial-arm-metric groups (when sorted by map_key values), and drops `map_key` columns.

This diff makes it so that `map_key` columns are still present in the `df` attribute, showing their values for each kept row. This will make it easier to understand and model partially complete trials in the future.

[RfC] I've patched up a few tests that assert these dataframes have a specific form, but there don't seem to be any tests failing for functional reasons (since the unused map_key columns would just go unused if not needed). Putting this up as an RfC though in case others feel strongly that these columns should be dropped.

Reviewed By: Balandat

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

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

bernardbeckerman pushed a commit to bernardbeckerman/Ax that referenced this pull request Aug 26, 2024
Summary:
`MapData` has an attribute `df` that takes only the last row from each of `map_df`'s trial-arm-metric groups (when sorted by map_key values), and drops `map_key` columns.

This diff makes it so that `map_key` columns are still present in the `df` attribute, showing their values for each kept row. This will make it easier to understand and model partially complete trials in the future.

[RfC] I've patched up a few tests that assert these dataframes have a specific form, but there don't seem to be any tests failing for functional reasons (since the unused map_key columns would just go unused if not needed). Putting this up as an RfC though in case others feel strongly that these columns should be dropped.

Reviewed By: Balandat

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

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

bernardbeckerman pushed a commit to bernardbeckerman/Ax that referenced this pull request Aug 26, 2024
Summary:
`MapData` has an attribute `df` that takes only the last row from each of `map_df`'s trial-arm-metric groups (when sorted by map_key values), and drops `map_key` columns.

This diff makes it so that `map_key` columns are still present in the `df` attribute, showing their values for each kept row. This will make it easier to understand and model partially complete trials in the future.

[RfC] I've patched up a few tests that assert these dataframes have a specific form, but there don't seem to be any tests failing for functional reasons (since the unused map_key columns would just go unused if not needed). Putting this up as an RfC though in case others feel strongly that these columns should be dropped.

Reviewed By: Balandat

Differential Revision: D61730570
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.28%. Comparing base (36194d3) to head (8cb9d3c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2701      +/-   ##
==========================================
+ Coverage   95.27%   95.28%   +0.01%     
==========================================
  Files         493      493              
  Lines       47903    47903              
==========================================
+ Hits        45641    45646       +5     
+ Misses       2262     2257       -5     

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

Summary:
`MapData` has an attribute `df` that takes only the last row from each of `map_df`'s trial-arm-metric groups (when sorted by map_key values), and drops `map_key` columns.

This diff makes it so that `map_key` columns are still present in the `df` attribute, showing their values for each kept row. This will make it easier to understand and model partially complete trials in the future.

[RfC] I've patched up a few tests that assert these dataframes have a specific form, but there don't seem to be any tests failing for functional reasons (since the unused map_key columns would just go unused if not needed). Putting this up as an RfC though in case others feel strongly that these columns should be dropped.

Reviewed By: Balandat

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

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

facebook-github-bot pushed a commit that referenced this pull request Aug 26, 2024
Summary:
`MapData` has an attribute `df` that takes only the last row from each of `map_df`'s trial-arm-metric groups (when sorted by map_key values), and drops `map_key` columns.

This diff makes it so that `map_key` columns are still present in the `df` attribute, showing their values for each kept row. This will make it easier to understand and model partially complete trials in the future.

[RfC] I've patched up a few tests that assert these dataframes have a specific form, but there don't seem to be any tests failing for functional reasons (since the unused map_key columns would just go unused if not needed). Putting this up as an RfC though in case others feel strongly that these columns should be dropped.

Reviewed By: Balandat

Differential Revision: D61730570

fbshipit-source-id: 0f09b357e0382b48a42b61d5c0f5a63206736196
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants