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

C++ layer to implement Triu #3631

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

C++ layer to implement Triu #3631

wants to merge 8 commits into from

Conversation

protonu
Copy link
Collaborator

@protonu protonu commented Dec 21, 2024

This is a C++ API to implement triu. The PR (#3637) stacked on top of this used it to create a Python interface.

Another way of using this may be to use the components of triu such as iota, broadcast, le and where from Thunder directly bypassing the need for a C++ implementation. As future work this commit/PR may be removed in favor of a Thunder only implementation.

@protonu protonu force-pushed the pbasu_iota_experiment branch from 260141f to 5f82c65 Compare December 23, 2024 21:33
@protonu protonu force-pushed the pbasu_iota_experiment branch from 0b42b74 to b734bf6 Compare January 2, 2025 20:12
@protonu
Copy link
Collaborator Author

protonu commented Jan 2, 2025

!test

@protonu
Copy link
Collaborator Author

protonu commented Jan 2, 2025

!test

@protonu protonu changed the title [WIP] C++ layer to implement Triu C++ layer to implement Triu Jan 2, 2025
@protonu protonu requested a review from jacobhinkle January 2, 2025 20:57
@protonu protonu marked this pull request as ready for review January 2, 2025 20:57
csrc/ops/composite.cpp Show resolved Hide resolved
csrc/ops/composite.cpp Outdated Show resolved Hide resolved
csrc/ops/composite.cpp Outdated Show resolved Hide resolved
csrc/ops/composite.cpp Outdated Show resolved Hide resolved
tests/cpp/test_tensor_factories.cpp Outdated Show resolved Hide resolved
@protonu protonu requested a review from jacobhinkle January 2, 2025 21:20
@protonu
Copy link
Collaborator Author

protonu commented Jan 2, 2025

!test

tests/cpp/test_tensor_factories.cpp Outdated Show resolved Hide resolved
csrc/ops/composite.cpp Outdated Show resolved Hide resolved
auto tv_columns = iota(
tv->domain()->logical()[dims - 1]->extent(),
SimplifyingIrBuilder::mulExpr(
offset, IrBuilder::create<Val>(-1, DataType::Index)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: it might be clearer to just use a standard iota here then subtract the offset, or add/subtract it below where you are building the predicate. Here it is unclear what the args to iota are doing.

csrc/ops/composite.cpp Show resolved Hide resolved
@protonu protonu requested a review from jacobhinkle January 3, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants