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

Add interpreter for CollectiveBroadcastOp #1983

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

ghpvnist
Copy link
Member

@ghpvnist ghpvnist commented Feb 1, 2024

closes #1982

Copy link
Contributor

@mlevesquedion mlevesquedion left a comment

Choose a reason for hiding this comment

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

Change looks good but I'm confused about how replicaGroups / processGroups work. I will need to do some more digging into those before I can properly review.

stablehlo/tests/interpret/collective_broadcast.mlir Outdated Show resolved Hide resolved
stablehlo/reference/Ops.cpp Show resolved Hide resolved
stablehlo/reference/Ops.cpp Show resolved Hide resolved
stablehlo/tests/interpret/collective_broadcast.mlir Outdated Show resolved Hide resolved
stablehlo/reference/Ops.cpp Show resolved Hide resolved
stablehlo/reference/Ops.cpp Outdated Show resolved Hide resolved
@ghpvnist ghpvnist mentioned this pull request Feb 6, 2024
@ghpvnist ghpvnist assigned GleasonK and ghpvnist and unassigned mlevesquedion and GleasonK Feb 6, 2024
@ghpvnist ghpvnist merged commit 36189e2 into openxla:main Feb 6, 2024
10 checks passed
@ghpvnist ghpvnist deleted the collective_broadcast branch February 6, 2024 19:41
ghpvnist added a commit that referenced this pull request Feb 6, 2024
As pointed out by @mlevesquedion in
#1983 (comment),
the symbol visibility is public by default, and making it implicit
follows the precedent of other tests in this directory.
copybara-service bot pushed a commit to openxla/xla that referenced this pull request Feb 6, 2024
This mirrors the upstream PR: openxla/stablehlo#1983

PiperOrigin-RevId: 604745617
mlevesquedion pushed a commit that referenced this pull request Feb 7, 2024
This seems to be from #1987
(see
https://github.com/openxla/stablehlo/actions/runs/7808382192/job/21298492956).

Not sure how presubmit failed to catch this. I think what happened is
the following:

1. #1987 was uploaded and
passed presubmit.
2. #1983 was submitted, adding
a new use of `makeScalar`.
3. #1987 was submitted,
deleting `makeScalar` and causing tests to fail at HEAD.

What I don't understand is why this sequence of operations is possible
in the first place. I feel like when
#1983 was submitted, the
earlier passing presubmit on
#1987 should have been
invalidated, blocking submit. I guess presubmits only re-run if you add
a commit? So in this case since there was no new commit and no conflict,
no new commit needed to be pushed and the presubmits were not run again.

In the grand scheme of things, I think this is fairly minor, but we may
want to investigate whether we can prevent this somehow by tweaking some
configuration settings or something.
copybara-service bot pushed a commit to openxla/xla that referenced this pull request Feb 7, 2024
This mirrors the upstream PR: openxla/stablehlo#1983

PiperOrigin-RevId: 604745617
copybara-service bot pushed a commit to openxla/xla that referenced this pull request Feb 7, 2024
This mirrors the upstream PR: openxla/stablehlo#1983

PiperOrigin-RevId: 604745617
copybara-service bot pushed a commit to openxla/xla that referenced this pull request Feb 7, 2024
This mirrors the upstream PR: openxla/stablehlo#1983

PiperOrigin-RevId: 604745617
copybara-service bot pushed a commit to tensorflow/mlir-hlo that referenced this pull request Feb 8, 2024
This mirrors the upstream PR: openxla/stablehlo#1983

PiperOrigin-RevId: 605152611
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Feb 8, 2024
This mirrors the upstream PR: openxla/stablehlo#1983

PiperOrigin-RevId: 605152611
copybara-service bot pushed a commit to openxla/xla that referenced this pull request Feb 8, 2024
This mirrors the upstream PR: openxla/stablehlo#1983

PiperOrigin-RevId: 605152611
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add interpreter for CollectiveBroadcastOp
3 participants