Skip to content

[Data] Set default strategy for callable class in get_compute_strategy#57657

Merged
bveeramani merged 5 commits intoray-project:masterfrom
owenowenisme:data/set-default-compute-strategy
Oct 15, 2025
Merged

[Data] Set default strategy for callable class in get_compute_strategy#57657
bveeramani merged 5 commits intoray-project:masterfrom
owenowenisme:data/set-default-compute-strategy

Conversation

@owenowenisme
Copy link
Copy Markdown
Member

@owenowenisme owenowenisme commented Oct 13, 2025 •
edited
Loading

Why are these changes needed?

Instead of raising ValueError when compute and concurrency is unset for callable class, we use ActorPoolStrategy(min_size=1, max_size=None) as default value.

Related issue number

Follow up for #57035

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run pre-commit jobs to lint the changes in this PR. (pre-commit setup)
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@owenowenisme
update
8c1fac0 
Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
@owenowenisme owenowenisme added the go add ONLY when ready to merge, run all tests label Oct 13, 2025
@owenowenisme
remove value error when not setting concurrency
7adc583 
Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
@owenowenisme owenowenisme marked this pull request as ready for review October 13, 2025 05:24
@owenowenisme owenowenisme requested a review from a team as a code owner October 13, 2025 05:24
@ray-gardener ray-gardener bot added the data Ray Data-related issues label Oct 13, 2025
bveeramani
bveeramani reviewed Oct 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bveeramani bveeramani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might need to update this too: https://github.com/owenowenisme/ray/blob/7adc5836a5df75a00d76f77f590a10a136b8a4f2/python/ray/data/_internal/util.py#L576-L584.

@owenowenisme
Copy link
Copy Markdown
Member Author

owenowenisme commented Oct 15, 2025

@bveeramani
Shouldn't this return error?
User should not set task strategy on callable class
I think this should raise error instead of setting implicit value.

@bveeramani
Copy link
Copy Markdown
Member

bveeramani commented Oct 15, 2025

@bveeramani Shouldn't this return error? User should not set task strategy on callable class I think this should raise error instead of setting implicit value.

Ah, yeah, good call.

I think we could be more precise in the error message. The error message says you need to specify a compute when using callable classes, but that isn't entirely true. You just can't use TaskPoolStrategy with callable classes.

Maybe we do something like this instead:

You specified the callable class {...} as your UDF with the compute {...}, but Ray Data can't schedule callable classes with the task pool compute. To fix this error, pass None or an ActorPoolStrategy to compute.

@owenowenisme
update error msg and add args description for default actor pool stra…
67d223a 
…tegy

Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
@owenowenisme
update
74ed525 
Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
bveeramani
bveeramani approved these changes Oct 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bveeramani bveeramani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@bveeramani bveeramani merged commit ca879fe into ray-project:master Oct 15, 2025
6 checks passed
justinyeh1995 pushed a commit to justinyeh1995/ray that referenced this pull request Oct 20, 2025
@owenowenisme @justinyeh1995
[Data] Set default strategy for callable class in `get_compute_strate…
0bd432a 
…gy` (ray-project#57657)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
Instead of raising ValueError when `compute` and `concurrency` is unset
for callable class, we use `ActorPoolStrategy(min_size=1,
max_size=None)` as default value.
<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number
Follow up for ray-project#57035
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run pre-commit jobs to lint the changes in this PR.
([pre-commit
setup](https://docs.ray.io/en/latest/ray-contribute/getting-involved.html#lint-and-formatting))
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
xinyuangui2 pushed a commit to xinyuangui2/ray that referenced this pull request Oct 22, 2025
@owenowenisme @xinyuangui2
[Data] Set default strategy for callable class in `get_compute_strate…
7f3ec11 
…gy` (ray-project#57657)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
Instead of raising ValueError when `compute` and `concurrency` is unset
for callable class, we use `ActorPoolStrategy(min_size=1,
max_size=None)` as default value.
<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number
Follow up for ray-project#57035
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run pre-commit jobs to lint the changes in this PR.
([pre-commit
setup](https://docs.ray.io/en/latest/ray-contribute/getting-involved.html#lint-and-formatting))
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
Signed-off-by: xgui <xgui@anyscale.com>
elliot-barn pushed a commit that referenced this pull request Oct 23, 2025
@owenowenisme @elliot-barn
[Data] Set default strategy for callable class in `get_compute_strate…
9b3d862 
…gy` (#57657)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
Instead of raising ValueError when `compute` and `concurrency` is unset
for callable class, we use `ActorPoolStrategy(min_size=1,
max_size=None)` as default value.
<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number
Follow up for #57035
<!-- For example: "Closes #1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run pre-commit jobs to lint the changes in this PR.
([pre-commit
setup](https://docs.ray.io/en/latest/ray-contribute/getting-involved.html#lint-and-formatting))
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
@owenowenisme @landscapepainter
[Data] Set default strategy for callable class in `get_compute_strate…
eb5cdf4 
…gy` (ray-project#57657)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
Instead of raising ValueError when `compute` and `concurrency` is unset
for callable class, we use `ActorPoolStrategy(min_size=1,
max_size=None)` as default value.
<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number
Follow up for ray-project#57035
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run pre-commit jobs to lint the changes in this PR.
([pre-commit
setup](https://docs.ray.io/en/latest/ray-contribute/getting-involved.html#lint-and-formatting))
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request Nov 19, 2025
@owenowenisme @Aydin-ab
[Data] Set default strategy for callable class in `get_compute_strate…
71b0152 
…gy` (ray-project#57657)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
Instead of raising ValueError when `compute` and `concurrency` is unset
for callable class, we use `ActorPoolStrategy(min_size=1,
max_size=None)` as default value.
<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number
Follow up for ray-project#57035
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run pre-commit jobs to lint the changes in this PR.
([pre-commit
setup](https://docs.ray.io/en/latest/ray-contribute/getting-involved.html#lint-and-formatting))
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request Dec 7, 2025
@owenowenisme @Future-Outlier
[Data] Set default strategy for callable class in `get_compute_strate…
23e2d66 
…gy` (ray-project#57657)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?
Instead of raising ValueError when `compute` and `concurrency` is unset
for callable class, we use `ActorPoolStrategy(min_size=1,
max_size=None)` as default value.
<!-- Please give a short summary of the change and the problem this
solves. -->

## Related issue number
Follow up for ray-project#57035
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run pre-commit jobs to lint the changes in this PR.
([pre-commit
setup](https://docs.ray.io/en/latest/ray-contribute/getting-involved.html#lint-and-formatting))
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: You-Cheng Lin (Owen) <mses010108@gmail.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bveeramani bveeramani bveeramani approved these changes

Assignees

No one assigned

Labels

data Ray Data-related issues go add ONLY when ready to merge, run all tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@owenowenisme @bveeramani