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 interval argument to LocalCartesian #7533

Merged
merged 5 commits into from
Jun 20, 2023
Merged

Conversation

mzamini92
Copy link
Contributor

The intermediate tensors cart and max_value in the original code were replaced with in-place operations to reduce memory usage. This was done by directly operating on the cart tensor and computing the maximum value iteratively without creating a separate max_value tensor. In-place operations (torch.sub, cart.div_, cart.mul_, cart.add_) were used to perform computations directly on tensors, reducing memory usage and eliminating the need for intermediate tensors. To compute the maximum value in a streaming fashion, a loop was introduced which iterates over the edges and updates the maximum value tensor (max_value) accordingly.

The intermediate tensors `cart` and `max_value` in the original code were replaced with in-place operations to reduce memory usage. This was done by directly operating on the `cart` tensor and computing the maximum value iteratively without creating a separate `max_value` tensor.
In-place operations `(torch.sub, cart.div_, cart.mul_, cart.add_)` were used to perform computations directly on tensors, reducing memory usage and eliminating the need for intermediate tensors.
To compute the maximum value in a streaming fashion, a loop was introduced in the combined version. This loop iterates over the edges and updates the maximum value tensor (max_value) accordingly.
@mzamini92 mzamini92 requested a review from wsad1 as a code owner June 6, 2023 17:19
@rusty1s rusty1s changed the title Update local_cartesian.py Add interval argument to LocalCartesian Jun 20, 2023
Copy link
Member

@rusty1s rusty1s left a comment

Choose a reason for hiding this comment

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

Thanks. I like the interval argument and I think it would be great to add this to other transforms as well, so please feel free to go ahead and modify other transforms.

Note that I removed the in-place refactoring in this PR. I don't think this is needed, and there were some for-loops added here that unnecessarily slowed down this transformation. Hope this works for you!

@rusty1s rusty1s enabled auto-merge (squash) June 20, 2023 13:11
@rusty1s rusty1s merged commit f5ebe30 into pyg-team:master Jun 20, 2023
mzamini92 added a commit to mzamini92/pytorch_geometric that referenced this pull request Sep 30, 2023
Following to [PR5733](pyg-team#7533)  I've Added the interval argument. @rusty1s With these modifications, both the Compose and ComposeFilters classes accept an interval argument, and it will be passed to the transformed data when composing transforms or applying filters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants