Skip to content

Conversation

@xinyuangui2
Copy link
Contributor

@xinyuangui2 xinyuangui2 commented Aug 17, 2025

Why are these changes needed?

This PR moves the implementations of collectives to TrainFnUtils. This would unblock the local mode that is introduced in #55487

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • 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: xgui <xgui@anyscale.com>
@xinyuangui2 xinyuangui2 requested a review from a team as a code owner August 17, 2025 18:01
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors collective operations like barrier and broadcast_from_rank_zero by moving their implementations from the public API module ray.train.collective.collectives to the internal TrainFnUtils class. This is a good architectural improvement that cleans up the public API surface and centralizes training-related utilities. The tests have been updated to reflect these changes. The changes are logical and well-executed. I have a couple of minor suggestions to improve code clarity and avoid redundant function calls.

@ray-gardener ray-gardener bot added the train Ray Train Related Issue label Aug 17, 2025
xinyuangui2 and others added 2 commits August 18, 2025 00:30
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Xinyuan <43737116+xinyuangui2@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Xinyuan <43737116+xinyuangui2@users.noreply.github.com>
@xinyuangui2 xinyuangui2 requested a review from justinvyu August 18, 2025 17:36
@@ -0,0 +1,56 @@
import logging
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: move to collective folder as per @justinvyu 's comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed offline and it made more sense to put inside the _internal/execution

Signed-off-by: xgui <xgui@anyscale.com>
@justinvyu justinvyu enabled auto-merge (squash) August 22, 2025 18:45
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Aug 22, 2025
@justinvyu justinvyu merged commit b0b77f6 into ray-project:master Aug 22, 2025
7 checks passed
jugalshah291 pushed a commit to jugalshah291/ray_fork that referenced this pull request Sep 11, 2025
…t#55689)

This PR moves the implementations of collectives to `TrainFnUtils`. This
would unblock the local mode that is introduced in
ray-project#55487

---------

Signed-off-by: xgui <xgui@anyscale.com>
Signed-off-by: Xinyuan <43737116+xinyuangui2@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>
dstrodtman pushed a commit that referenced this pull request Oct 6, 2025
This PR moves the implementations of collectives to `TrainFnUtils`. This
would unblock the local mode that is introduced in
#55487

---------

Signed-off-by: xgui <xgui@anyscale.com>
Signed-off-by: Xinyuan <43737116+xinyuangui2@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Douglas Strodtman <douglas@anyscale.com>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
…t#55689)

This PR moves the implementations of collectives to `TrainFnUtils`. This
would unblock the local mode that is introduced in
ray-project#55487

---------

Signed-off-by: xgui <xgui@anyscale.com>
Signed-off-by: Xinyuan <43737116+xinyuangui2@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests train Ray Train Related Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants