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

[Tune] Keep resource specifications when nesting with_resources in with_parameters #29740

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions python/ray/tune/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1530,6 +1530,34 @@ def train_fn(self, config):
assert trial.last_result["_metric"] == num_gpus


@pytest.mark.parametrize("num_gpus", [1, 2])
def test_with_resources_and_parameters_fn(ray_start_2_cpus_2_gpus, num_gpus):
def train_fn(config, extra_param=None):
assert extra_param is not None, "Missing extra parameter."
print(ray.get_runtime_context().get_assigned_resources())
return {"num_gpus": len(ray.get_gpu_ids())}

# Nesting `tune.with_parameters` and `tune.with_resources` should respect
# the resource specifications.
trainable = tune.with_resources(
tune.with_parameters(train_fn, extra_param="extra"),
{"gpu": num_gpus},
)

tuner = tune.Tuner(trainable)
results = tuner.fit()
print(results[0].metrics)
assert results[0].metrics["num_gpus"] == num_gpus

# The other order of nesting should work the same.
trainable = tune.with_parameters(
tune.with_resources(train_fn, {"gpu": num_gpus}), extra_param="extra"
)
tuner = tune.Tuner(trainable)
results = tuner.fit()
assert results[0].metrics["num_gpus"] == num_gpus


class SerializabilityTest(unittest.TestCase):
@classmethod
def setUpClass(cls):
Expand Down
13 changes: 11 additions & 2 deletions python/ray/tune/trainable/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,19 +362,28 @@ def inner(config, checkpoint_dir=None):

for k in keys:
fn_kwargs[k] = parameter_registry.get(prefix + k)
trainable(config, **fn_kwargs)
return trainable(config, **fn_kwargs)

inner.__name__ = trainable_name

# If the trainable has been wrapped with `tune.with_resources`, we should
# keep the `_resources` attribute around
if hasattr(trainable, "_resources"):
inner._resources = trainable._resources

# Use correct function signature if no `checkpoint_dir` parameter
# is set
if not use_checkpoint:

def _inner(config):
inner(config, checkpoint_dir=None)
return inner(config, checkpoint_dir=None)

_inner.__name__ = trainable_name

# Again, pass along the resource specification if it exists
if hasattr(inner, "_resources"):
_inner._resources = inner._resources

if hasattr(trainable, "__mixins__"):
_inner.__mixins__ = trainable.__mixins__
return _inner
Expand Down