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

Removed create_golden_Tensor, created "safe" custom Constructor #2136

Merged
merged 5 commits into from
Feb 19, 2025

Conversation

vprajapati-tt
Copy link
Contributor

  • PR closes Python/Passes.cpp - create_golden_tensor has unsafe pointer reference #2027
  • Changes container for GoldenTensor in Python to be a shared_ptr, ensuring data is maintained and not copied.
  • Uses GoldenTensor custom constructor instead of create_golden_tensor.
  • Creates copy of supplied data_ptr (size of data must also be provided, example in ttir_builder.py) and stores within GoldenTensor object.

@vprajapati-tt vprajapati-tt changed the title Vprajapati/issue 2027 Removed create_golden_Tensor, created "safe" custom Constructor Feb 6, 2025
@vprajapati-tt
Copy link
Contributor Author

@vroubtsovTT I tried to make the vector a unique_ptr but pybind kept trying to keep a copy of it. Let me know if you have any thoughts on this and what I could've changed / done better. Thanks :)

@vroubtsovTT
Copy link
Contributor

vroubtsovTT commented Feb 7, 2025

@vroubtsovTT I tried to make the vector a unique_ptr but pybind kept trying to keep a copy of it. Let me know if you have any thoughts on this and what I could've changed / done better. Thanks :)

That may have been a sign that you were on the right track. Somebody wanted to copy your struct and couldn't because std::unique_ptr member fields aren't copyable -- this failed as it should. But a std::unique_ptr member will be move-copyable... you should try to make your struct move-copyable by adding

MyStruct(MyStruct && rhs) = default;

(and/or implementing such a move constructor if default doesn't work).

Copy link
Contributor

@odjuricicTT odjuricicTT left a comment

Choose a reason for hiding this comment

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

Explorer changes LGTM

@vprajapati-tt vprajapati-tt merged commit 4235b1f into main Feb 19, 2025
32 checks passed
@vprajapati-tt vprajapati-tt deleted the vprajapati/issue-2027 branch February 19, 2025 16:34
ctodTT added a commit that referenced this pull request Feb 19, 2025
Closes #1961
Closes #1759

### Problem description
`transpose` wasn't exposed to `ttir_builder`, preventing llama attention
layer modelling

### Special Thanks
To @vprajapati-tt for inadvertently fixing a blocking memory issue that
was exposed by this bug (See #2136 )
vwellsTT pushed a commit that referenced this pull request Feb 20, 2025
- PR closes #2027 
- Changes container for `GoldenTensor` in Python to be a shared_ptr,
ensuring data is maintained and not copied.
- Uses `GoldenTensor` custom constructor instead of
`create_golden_tensor`.
- Creates copy of supplied data_ptr (size of data must also be provided,
example in `ttir_builder.py`) and stores within GoldenTensor object.
vwellsTT pushed a commit that referenced this pull request Feb 20, 2025
Closes #1961
Closes #1759

### Problem description
`transpose` wasn't exposed to `ttir_builder`, preventing llama attention
layer modelling

### Special Thanks
To @vprajapati-tt for inadvertently fixing a blocking memory issue that
was exposed by this bug (See #2136 )
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.

Python/Passes.cpp - create_golden_tensor has unsafe pointer reference
4 participants