-
Notifications
You must be signed in to change notification settings - Fork 357
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
fix: convert invalid hparam types to json string #9449
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9449 +/- ##
==========================================
- Coverage 49.00% 48.99% -0.01%
==========================================
Files 1234 1234
Lines 159671 159677 +6
Branches 2779 2778 -1
==========================================
- Hits 78248 78240 -8
- Misses 81248 81262 +14
Partials 175 175
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
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
master/internal/db/postgres_trial.go
Outdated
valBytes, err := json.Marshal(v) | ||
if err != nil { | ||
return hparamsModel, projHparamsModel, | ||
fmt.Errorf("cannot assign hyperparameter %s, failed to encode type %T", hpName, val) |
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.
include the json error here
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.
Added.
Ticket
ET-292
Description
Instead of throwing an error, we want to convert run hparams that are not text,numbers or dates into strings before being added to the run_hparam perf table
Test Plan
This is a fix for the nightly tests. Specifically
test-e2e-mmdetection
. This test should no longer failChecklist
docs/release-notes/
.See Release Note for details.