Skip to content

Simple PyTorch extension API #1247

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

Merged
merged 2 commits into from
Mar 31, 2025

Conversation

vloncar
Copy link
Contributor

@vloncar vloncar commented Mar 26, 2025

Description

The equivalent of Keras extension API for PyTorch. The least hacky way of doing it, but only limited to custom modules. The custom modules should extend hls4ml.utils.torch.HLS4MLModule (itself a torch.nn.Module). I borrowed the CustomFXTracer change from #1019 and moved it to a different location, since the file contains HLS4MLModule class intended to be used by the user.

Type of change

  • New feature (non-breaking change which adds functionality)

Tests

Test is included in test_extensions_pytorch.py

Checklist

  • I have read the guidelines for contributing.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have installed and run pre-commit on the files I edited or added.
  • I have added tests that prove my fix is effective or that my feature works.

@vloncar vloncar requested a review from JanFSchulte March 26, 2025 20:14
first = node.get_input_node()
second = node

model.remove_node(first, rewire=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the rewire parameter doesn't do anything anymore, might be good to just not include it here if people end up using this as an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have probably copy/pasted that line from one of the other optimizers. Should I replace all instances of this? I see there's a few around

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to clean this up in general. I'm not sure why we have this around at all anymore, since the parameter is really just a dummy according to https://github.com/fastmachinelearning/hls4ml/blob/main/hls4ml/model/graph.py#L520-L537

@JanFSchulte JanFSchulte merged commit 0460695 into fastmachinelearning:main Mar 31, 2025
4 of 5 checks passed
@vloncar vloncar deleted the pytorch_extension_api branch March 31, 2025 20:17
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