-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Split out model vs microbatch execution #10737
Split out model vs microbatch execution #10737
Conversation
…l execution In commit 8fe5ea1 the adding of the relationship to the adapter cache got moved to _before_ the post hook during model execution. This was done because of some wonkiness of having the microbatch model and model execution logic in the same place. Now that we've split the relevant logic, we can move the caching back to it's original spot in the order of operations. We don't think this actually changes anything, but we're moving it back juuuust in case because this is a very hot code path.
dfb5076
to
89f1c67
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10737 +/- ##
==========================================
- Coverage 89.00% 88.95% -0.05%
==========================================
Files 181 181
Lines 22964 22971 +7
==========================================
- Hits 20440 20435 -5
- Misses 2524 2536 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The failing unit test, |
Resolves #N/A
Description
In #10677 we added microbatch model execution logic to the
ModelRunner.execute
. In doing so we changed the order of operations of some normal model execution steps. This PR splits the logic so that we can return the order of operations for normal model execution back to what it was previously while still supporting microbatch model execution. We don't know if the change of order operations that happened in #10677 was problematic, but moving it back is being done out caution.Checklist