Skip to content

Conversation

@sanjibansg
Copy link
Contributor

@sanjibansg sanjibansg commented Apr 10, 2025

This PR adds two separate optimization modes in SOFIE so as to control the level of optimization the user prefers. As of now, we have OptimizationLevel:kBasic and OptimizationLevel:kExtended. By default, OptimizationLevel:kExtended is enabled which can be changed by:

using namespace TMVA::Experimental;
SOFIE::RModelParser_ONNX parser;
SOFIE::RModel model = parser.Parse("model.onnx");
model.SetOptimizationLevel(SOFIE::OptimizationLevel::kBasic);
model.Generate();

Optimization levels with their corresponding flags:

  • kBasic: Operator fusion (for example: fusing ReLU with a preceeding GEMM)
  • kExtended: Reuse memory for intermediate tensors

@sanjibansg sanjibansg requested a review from lmoneta as a code owner April 10, 2025 17:00
@github-actions
Copy link

github-actions bot commented Apr 10, 2025

Test Results

    17 files      17 suites   4d 12h 53m 9s ⏱️
 2 738 tests  2 735 ✅ 0 💤 3 ❌
44 943 runs  44 940 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit bd16854.

♻️ This comment has been updated with latest results.

@sanjibansg sanjibansg force-pushed the sofie/memory/optional_optimization branch from ec6e64a to 8529996 Compare April 14, 2025 10:37
Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR!

I have validated that this works now for our use case of doing reverse mode AD with Clad on the emitted code.

Maybe consider squashing the commits on merge, since the first commit is not a state where the tests would pass and the second commit is a fixup to the first commit (let's try to make the tests pass at every commit).

Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

LGTM!

@dpiparo dpiparo merged commit 5a08e75 into root-project:master Apr 15, 2025
18 of 22 checks passed
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.

4 participants