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.

Sign up for GitHub

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

Jump to bottom

Add a tensor_mul operator #114

Merged
merged 45 commits into from
Mar 4, 2025
Merged

Add a tensor_mul operator #114

merged 45 commits into from
Mar 4, 2025

Conversation

EmilyBourne
Copy link
Member

Add a tensor_mul operator. Fixes #76

@EmilyBourne EmilyBourne force-pushed the ebourne-76-tensor-mul-operator branch from 85dec89 to 088b246 Compare February 25, 2025 07:46
@EmilyBourne EmilyBourne force-pushed the ebourne-76-tensor-mul-operator branch from caa9764 to 4bd2440 Compare February 25, 2025 14:02
@EmilyBourne EmilyBourne marked this pull request as ready for review February 26, 2025 14:12
@EmilyBourne EmilyBourne marked this pull request as ready for review February 28, 2025 16:46
@EmilyBourne EmilyBourne added the Ready to review Label to be automatically added to a PR when it is ready to be reviewed label Feb 28, 2025
@EmilyBourne
Copy link
Member Author

Are these functions meant to be callable on the device ? If so they would need a KOKKOS_FUNCTION annotation.

Fixed 3e57619

tpadioleau
tpadioleau reviewed Mar 3, 2025
View reviewed changes
Copy link
Member

@tpadioleau tpadioleau left a comment

Choose a reason for hiding this comment

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

almost good to me

src/data_types/indexed_tensor.hpp
"contravariant indices.");

private:
TensorType& m_tensor;
Copy link
Member

Choose a reason for hiding this comment

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

Yes that will avoid the copy. I think we adhere to the rules of zero and five ? If we define one of them we must define them all. I think we also want to delete the copy and move semantic.

@EmilyBourne EmilyBourne temporarily deployed to GitLab GPU trigger March 4, 2025 08:05 — with GitHub Actions Inactive
tpadioleau
tpadioleau requested changes Mar 4, 2025
View reviewed changes
@tpadioleau
Copy link
Member

After that we are good

@EmilyBourne EmilyBourne temporarily deployed to GitLab GPU trigger March 4, 2025 15:24 — with GitHub Actions Inactive
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 4, 2025

It seems like you haven't finished working on this PR so it has been put back into draft. Please remove the draft status when the PR can run tests without being interrupted.

@github-actions github-actions bot marked this pull request as draft March 4, 2025 15:25
@github-actions github-actions bot removed the Ready to review Label to be automatically added to a PR when it is ready to be reviewed label Mar 4, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 4, 2025

This PR is failing tests so it has been put back into draft. Please remove the draft status when the tests pass.

1 similar comment
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 4, 2025

This PR is failing tests so it has been put back into draft. Please remove the draft status when the tests pass.

@EmilyBourne EmilyBourne marked this pull request as ready for review March 4, 2025 15:26
@EmilyBourne EmilyBourne requested a review from tpadioleau March 4, 2025 15:26
@EmilyBourne EmilyBourne temporarily deployed to GitLab GPU trigger March 4, 2025 15:27 — with GitHub Actions Inactive
tpadioleau
tpadioleau approved these changes Mar 4, 2025
View reviewed changes
Copy link
Member

@tpadioleau tpadioleau left a comment

Choose a reason for hiding this comment

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

Let's merge that

@EmilyBourne EmilyBourne enabled auto-merge (squash) March 4, 2025 15:46
@EmilyBourne EmilyBourne added the Ready to merge Label to be automatically added to a PR when it is approved and tests are passing label Mar 4, 2025
@EmilyBourne EmilyBourne merged commit fc1c4f9 into main Mar 4, 2025
33 checks passed
@EmilyBourne EmilyBourne deleted the ebourne-76-tensor-mul-operator branch March 4, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanmnc yanmnc yanmnc left review comments

@tpadioleau tpadioleau tpadioleau approved these changes

Assignees
No one assigned
Labels
Ready to merge Label to be automatically added to a PR when it is approved and tests are passing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create a tensor_mul operator
3 participants
@EmilyBourne @tpadioleau @yanmnc