-
Notifications
You must be signed in to change notification settings - Fork 12
Add ONNX benchmark #381
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
Add ONNX benchmark #381
Conversation
m_fb = factory::fb_factory<sopt::algorithm::ImagingForwardBackward<t_complex>>( | ||
factory::algo_distribution::mpi_serial, m_measurements_distribute_grid, wavelets, m_uv_data, | ||
m_sigma, beta, gamma, m_imsizey, m_imsizex, m_sara.size(), state.range(3), true, true, false, | ||
1e-3, 1e-2, 50, tf_model_path, nondiff_func_type::Denoiser); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the measurement operator normalisation calculated anywhere? It doesn't appear to be in AlgoFixtureMPI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No? I'd assume it just uses the default 1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine as long as we can confirm that the measurement operator is being normalised properly (if that's necessary for this benchmark), and if we don't need the code in the if(false)
statement then we should probably just delete it.
Something is broken with homebrew on the MacOS runners 😞 Today it works 🥳 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than a parameter change in one of the benchmarks, I don't see any problems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addresses the changes I requested.
Ubuntu CI is failing due to actions/runner-images#11926 |
No description provided.