Skip to content

[Transform] Construct on GPU, cache on CPU #352

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

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

Conversation

kylesayrs
Copy link
Contributor

@kylesayrs kylesayrs commented Jun 11, 2025

Purpose

  • Enable construction of large Hadamard matrices, whose large sizes require construction on the execution device (GPU)

Prerequisites

Changes

  • Extend ParameterizedDefaultDict to pass keyword arguments to factory function
  • Modify hadamard factories to construct hadamards on the execution device (GPU), but cache them on the offload device (CPU)

Testing

  • Transform correctness and memory management tests pass

kylesayrs added 13 commits June 11, 2025 13:45
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs kylesayrs changed the base branch from main to kylesayrs/extend-hadamard June 11, 2025 21:57
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
…gin' into kylesayrs/transform_construct_cache_device
@kylesayrs kylesayrs marked this pull request as ready for review June 11, 2025 22:08
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Copy link
Contributor

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

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

everything makes sense to me but the new method on ParametrizedDefaultDict, left a comment

Comment on lines +386 to +388
def get(self, *args, **kwargs) -> Any:
with patch_attr(self, "_kwargs", kwargs):
return self[args]
Copy link
Contributor

Choose a reason for hiding this comment

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

a docstring might be useful here. basically kwargs are only used if the key to get is missing, and user has to know args are keys and kwargs are constructor inputs? Maybe something like

def get(self, *args, constructor_kwargs={}):

would be a clearer signature?

Copy link
Contributor Author

@kylesayrs kylesayrs Jun 12, 2025

Choose a reason for hiding this comment

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

I'll admit that the current implementation is less verbose than it could be for a somewhat unfamiliar class

Base automatically changed from kylesayrs/extend-hadamard to main June 13, 2025 16:44
@dsikka dsikka dismissed brian-dellabetta’s stale review June 13, 2025 16:44

The base branch was changed.

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