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

Onnx implementation #2

Merged
merged 12 commits into from
Mar 27, 2024
Merged

Onnx implementation #2

merged 12 commits into from
Mar 27, 2024

Conversation

nprouvost
Copy link
Collaborator

Implementation of ONNX, implementation of PlotRuntimesMultipleParams instead of PlotRuntimesMultipleNetworks and PlotRuntimesMultipleCMSSW, refactoring parameters and new parameters for labels, add onnx examples and update existing tf examples, updated docu (wrong as of now in master after last refactoring)

@nprouvost nprouvost added documentation Improvements or additions to documentation enhancement New feature or request labels Feb 6, 2024
@nprouvost nprouvost requested a review from riga February 6, 2024 15:57
Copy link
Member

@riga riga left a comment

Choose a reason for hiding this comment

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

Very nice!

I have two minor comments (inline) and two additional suggestions.

  1. There is now quite a lot of code duplication between the ONNX and TF runtime measurement plugins. Let's have the overlap organized in a common base class.
  2. We should also be able to load models from yaml files instead of json.

Let's chat privately on how to divide the work.

mlprof/tasks/parameters.py Outdated Show resolved Hide resolved
mlprof/tasks/parameters.py Show resolved Hide resolved
Copy link
Member

@riga riga left a comment

Choose a reason for hiding this comment

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

LGTM!

@riga riga merged commit 54ba404 into master Mar 27, 2024
1 check passed
@riga riga deleted the onnx_implementation branch March 27, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants