Skip to content

Commit

Permalink
Change fetch_data to lookup_data in exp_to_df
Browse files Browse the repository at this point in the history
Summary: Calling fetch_data evaluates metrics, which we would of course like to avoid.

Reviewed By: lena-kashtelyan

Differential Revision: D34689470

fbshipit-source-id: ce3918ff3f5d8740e5588a296b11cf25c498a039
  • Loading branch information
mpolson64 authored and facebook-github-bot committed Mar 8, 2022
1 parent 50a4d32 commit f63ca11
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
5 changes: 3 additions & 2 deletions ax/service/tests/test_report_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def test_exp_to_df(self):
self.assertEqual(len(df.index), len(exp.arms_by_name))

exp.trials[0].run()
exp.fetch_data()

# assert result is df with expected columns and length
df = exp_to_df(exp=exp)
Expand Down Expand Up @@ -107,15 +108,15 @@ def test_exp_to_df(self):
}
)
)
with patch.object(Experiment, "fetch_data", lambda self, metrics: mock_results):
with patch.object(Experiment, "lookup_data", lambda self: mock_results):
df = exp_to_df(exp=exp)

# all but one row should have a metric value of NaN
self.assertEqual(pd.isna(df[OBJECTIVE_NAME]).sum(), len(df.index) - 1)

# an experiment with more results than arms raises an error
with patch.object(
Experiment, "fetch_data", lambda self, metrics: mock_results
Experiment, "lookup_data", lambda self: mock_results
), self.assertRaisesRegex(ValueError, "inconsistent experimental state"):
exp_to_df(exp=get_branin_experiment())

Expand Down
11 changes: 7 additions & 4 deletions ax/service/utils/report_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,6 @@ def exp_to_df(
in ``experiment.trials``. If there are multiple arms per trial, these
fields will be replicated across the arms of a trial. Output columns names
will be prepended with ``"trial_properties_"``.
**kwargs: Custom named arguments, useful for passing complex
objects from call-site to the `fetch_data` callback.
Returns:
DataFrame: A dataframe of inputs, metadata and metrics by trial and arm (and
Expand All @@ -459,6 +457,11 @@ def exp_to_df(
metadata.
"""

if len(kwargs) > 0:
logger.warn(
"`kwargs` in exp_to_df is deprecated. Please remove extra arguments."
)

# Accept Experiment and SimpleExperiment
if isinstance(exp, MultiTypeExperiment):
raise ValueError("Cannot transform MultiTypeExperiments to DataFrames.")
Expand All @@ -475,12 +478,12 @@ def exp_to_df(
)

# Fetch results; in case arms_df is empty, return empty results (legacy behavior)
data = exp.fetch_data(metrics, **kwargs)
data = exp.lookup_data()
results = data.df
if len(arms_df.index) == 0:
if len(results.index) != 0:
raise ValueError(
"exp.fetch_data().df returned more rows than there are experimental "
"exp.lookup_data().df returned more rows than there are experimental "
"arms. This is an inconsistent experimental state. Please report to "
"Ax support."
)
Expand Down

0 comments on commit f63ca11

Please sign in to comment.