-
Notifications
You must be signed in to change notification settings - Fork 354
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
perf: avoid loading model def in experiment model #8742
Conversation
✅ Deploy Preview for determined-ui canceled.
|
}, user, | ||
) | ||
if err != nil { | ||
return nil, fmt.Errorf("parsing continue experiment request: %w", err) | ||
} | ||
dbExp.ParentID = nil // Not actually a parent though. |
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.
the only parent logic was loading the model def which was unused
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #8742 +/- ##
=======================================
Coverage 47.72% 47.72%
=======================================
Files 1049 1049
Lines 167238 167238
Branches 2241 2243 +2
=======================================
Hits 79816 79816
Misses 87264 87264
Partials 158 158
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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
0f57762
to
9429267
Compare
Description
We often load model def for experiments when we really shouldn't. This change makes it a lot harder to load the model def if you don't mean to.
Test Plan
Existing tests pass
Commentary (optional)
Checklist
docs/release-notes/
.See Release Note for details.
Ticket