-
Notifications
You must be signed in to change notification settings - Fork 120
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
Profile AutoDiffCostFunction and refactor the homography example #296
Conversation
Can you post the timing info with and without functorch? |
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.
LGTM. In a future PR we can make those parameters
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.
Looks good to me
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.
Looks great, nice work on speeding this up Taosha!
TODOs flagged so far before merging
- Add the perf results to the PR description (can you also include relative improvement here in addition to the absolute numbers you have).
- Consolidate the original example and profiling files into a single one that can take
--profile
flag.
Finally, in another PR setup enum for AutoDiffCostFunction grad options: {autograd, loop_batch, vectorize}
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.
LGTM!
Ah sorry, we should also add the script for running the benchmarks: |
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.
Looks good! Just a few minor comments before we can merge.
|
||
|
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.
Remove extra lines?
586dde4
to
6ef3388
Compare
…ebookresearch#296) * add logging and functorch to homography example * add homography profiling script * backup * import cast from typing * add hydra for homography * update homography example * update homography_estimation.py with AutogradMode * update homography example * add profile_auto_diff_cost_function.sh * move profile_auto_diff_cost_function.sh to evaluations * add "time/per peoch" to homography_estimation * update evaluations/README.md * 1025->1024 * refactor readme * move comments * add a new line * add a condition to ignore warning for the homography example
Motivation and Context
Profile
AutoDiffCostFunction
with differentAutogradMode
on the homography example.How Has This Been Tested
Types of changes
Checklist