determined icon indicating copy to clipboard operation
determined copied to clipboard

fix: assign only run in a single run experiment as best_trial_id

Open corban-beaird opened this issue 11 months ago • 3 comments

Description

Currently if a single-run/trial experiment doesn't store validation metrics, then it will not be assigned a best_trial_id, so when there are single trial experiments that are using report_metrics and do not use the validation metrics group, features like experiment comparison are unable to display data.

This PR adds an additional trigger, and backfill logic, that runs on experiment completions that have no best_trial_id assigned. This will then populate the best_trial_id field if it's a single run/trial experiment.

Test Plan

  • automated test case added to master/internal/api_experiment_intg_test.go

Commentary (optional)

Checklist

  • [ ] Changes have been manually QA'd
  • [ ] User-facing API changes need the "User-facing API Change" label.
  • [ ] Release notes should be added as a separate file under docs/release-notes/. See Release Note for details.
  • [ ] Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

ET-115

corban-beaird avatar Mar 26 '24 16:03 corban-beaird

Deploy Preview for determined-ui canceled.

Name Link
Latest commit 669eaa887699b6ec7fe97cd51e7db9aa9fb7f0e1
Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/66a10d497532ae0008719b82

netlify[bot] avatar Mar 26 '24 16:03 netlify[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 57.40%. Comparing base (543380d) to head (669eaa8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9051      +/-   ##
==========================================
+ Coverage   53.57%   57.40%   +3.83%     
==========================================
  Files        1255      761     -494     
  Lines      152855   102902   -49953     
  Branches     3298     3298              
==========================================
- Hits        81885    59072   -22813     
+ Misses      70820    43680   -27140     
  Partials      150      150              
Flag Coverage Δ
backend 43.77% <ø> (-1.06%) :arrow_down:
harness 71.47% <ø> (-1.38%) :arrow_down:
web 52.03% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 510 files with indirect coverage changes

codecov[bot] avatar Mar 26 '24 16:03 codecov[bot]

Thank you for doing this! The description sound like the desired behavior, but I don't really know this part of the code base (or go or sql), so I will leave the technical review to someone else.

garrett361 avatar Mar 26 '24 16:03 garrett361

@corban-beaird what is the status of the PR?

bitfyre avatar Jul 11 '24 00:07 bitfyre

@corban-beaird what is the status of the PR?

Right now my focus is on 2 high priority items for CM, once these are complete I'll be coming back to this PR and resolving the CI failures. I should be looking at having those items taken care by the end of this week at the latest; so currently on track to get this in prior to the release branch being cut this upcoming Tuesday.

corban-beaird avatar Jul 11 '24 18:07 corban-beaird