Skip to content

Refactor fusing#1386

Merged
reuvenperetz merged 63 commits intomainfrom
refactor-fusing
Apr 2, 2025
Merged

Refactor fusing#1386
reuvenperetz merged 63 commits intomainfrom
refactor-fusing

Conversation

@reuvenperetz
Copy link
Copy Markdown
Contributor

Pull Request Description:

This PR introduces handling graph fusion by encapsulating fusion metadata within a new wrapper class for the graph. This class ensures that after every access to the graph, a validation check is performed to verify that the fusion information remains consistent and that no modifications have introduced inconsistencies. Additionally, the fusion-related logic has been refactored into a new class called GraphFuser, which takes the graph along with its fusion metadata and creates a new graph where fused operations are represented as single nodes.

Checklist before requesting a review:

  • I set the appropriate labels on the pull request.
  • I have added/updated the release note draft (if necessary).
  • I have updated the documentation to reflect my changes (if necessary).
  • All function and files are well documented.
  • All function and classes have type hints.
  • There is a licenses in all file.
  • The function and variable names are informative.
  • I have checked for code duplications.
  • I have added new unittest (if necessary).

@reuvenperetz reuvenperetz requested a review from ofirgo March 16, 2025 07:45
input_shape=nodes[0].input_shape,
output_shape=nodes[-1].output_shape,
weights={},
weights={}, # TODO: update with weights of all nodes
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the todo here planned for this PR?
is it necessary actually? because you can always retrieve the original weights from the original graph

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, in the PR that handles the MP.

Copy link
Copy Markdown
Contributor

@ofirgo ofirgo left a comment

Choose a reason for hiding this comment

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

consider adding an e2e tests to verify that none of our substitutions fails the fusing metadata.

@github-actions github-actions bot removed the auto:qat label Mar 26, 2025
@reuvenperetz reuvenperetz merged commit 7a08405 into main Apr 2, 2025
36 of 37 checks passed
@reuvenperetz reuvenperetz deleted the refactor-fusing branch April 2, 2025 14:03
lior-dikstein pushed a commit that referenced this pull request Apr 8, 2025
This commit introduces handling graph fusion by encapsulating fusion metadata in the graph. This ensures that after access to the graph that modify the graph, a validation check is performed to verify that the fusion information remains consistent and that no modifications have introduced inconsistencies. Additionally, the fusion-related logic has been refactored into a new class called GraphFuser, which takes the graph along with its fusion metadata and creates a new graph where fused operations are represented as single nodes.

---------

Co-authored-by: reuvenp <reuvenp@altair-semi.com>
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