feat: Add Run columns to GetProjectColumns #9146

AmanuelAaron · 2024-04-10T19:59:21Z

Description

Add support for Run Table Columns in GetProjectColumns endpoint

Test Plan

integration tests should pass

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-176

netlify · 2024-04-10T19:59:37Z

✅ Deploy Preview for determined-ui canceled.

Name	Link
🔨 Latest commit	`985eb54`
🔍 Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/664e2c0b5e3dd700087c8452

codecov · 2024-04-10T20:00:44Z

Codecov Report

Attention: Patch coverage is 48.89868% with 116 lines in your changes are missing coverage. Please review.

Project coverage is 48.59%. Comparing base (6ff8eb7) to head (985eb54).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #9146    +/-   ##
========================================
  Coverage   48.59%   48.59%            
========================================
  Files        1234     1234            
  Lines      158840   158992   +152     
  Branches     2779     2779            
========================================
+ Hits        77187    77267    +80     
- Misses      81478    81550    +72     
  Partials      175      175

Flag	Coverage Δ
backend	`42.15% <48.19%> (+0.02%)`	⬆️
harness	`64.04% <80.00%> (+<0.01%)`	⬆️
web	`44.35% <ø> (ø)`

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

Files	Coverage Δ
master/internal/db/postgres_trial_metrics.go	`91.53% <100.00%> (ø)`
master/pkg/model/experiment.go	`14.28% <ø> (ø)`
harness/determined/common/api/bindings.py	`40.22% <80.00%> (+0.02%)`	⬆️
master/internal/api_runs.go	`73.55% <81.48%> (-0.11%)`	⬇️
master/internal/experiment/bulk_action.go	`0.00% <0.00%> (ø)`
master/internal/api_project.go	`33.66% <63.44%> (+9.50%)`	⬆️
master/internal/db/postgres_trial.go	`63.10% <32.05%> (-4.34%)`	⬇️

... and 5 files with indirect coverage changes

master/internal/api_project.go

EmilyBonar

Make sure you add the following columns:

experimentProgress
experimentName (move from getProjectColumnsById after rebase)
experimentId
isExpMultitrial
parentArchived

master/internal/api_project.go

Not relevant

NicholasBlaskey · 2024-05-13T19:36:59Z

master/internal/api_project.go

+		MetricType string
+	}{}
+
+	if len(hyperparameters) > 0 {


what is this len check for?

We only want to do this query if the result query is not empty. Added a comment to clarify.

technically you could have a project without hyperparameters but have them have summary metrics

I think its a premature optimization

master/internal/api_project.go

master/internal/api_runs.go

master/internal/db/postgres_trial.go

master/static/migrations/20240508134054_add-run-hparam-table.tx.up.sql

Co-authored-by: Nick Blaskey <nick.blaskey@hpe.com>

… into project-columns

EmilyBonar

The web side of things looks good

NicholasBlaskey

lgtm

NicholasBlaskey · 2024-05-20T20:16:34Z

master/static/migrations/20240520104140_add-run-hparam-table.tx.up.sql

@@ -0,0 +1,42 @@
+CREATE TABLE run_hparams (


we drop projects.hyperparameters and delete the associated code now that it is replaced by this?

Can do this in a follow up ticket if you want

Created a new ticket to remove this: ET-268

master/internal/api_project.go

NicholasBlaskey · 2024-05-20T20:24:38Z

master/internal/api_project.go

+		MetricType string
+	}{}
+
+	if len(hyperparameters) > 0 {


technically you could have a project without hyperparameters but have them have summary metrics

I think its a premature optimization

NicholasBlaskey · 2024-05-20T20:32:31Z

performance/src/api_performance_tests.ts

+      !!sD?.workspace.projectId,
+    ),
+    test(
+      "Get Project Columns for Experiments Table",


is this the same endpoint as https://github.com/determined-ai/determined/blob/805f2f4ea5af73b2f141f33a7771a3c5e255e8d9/performance/src/api_performance_tests.ts#L196C44-L196C53?

is it going to slow down all the other performance tests while we fix it or has it been improved?

It is effectively the same call. I removed it as only the performance for the new columns have been improved.

AmanuelAaron added 2 commits April 10, 2024 15:12

project columns

28b8ab1

fixes + start testing

32e75ae

cla-bot bot added the cla-signed label Apr 10, 2024

AmanuelAaron added 3 commits April 10, 2024 16:42

add enum

ec9a694

lint

5c55fac

check column values

2920f2e

AmanuelAaron requested review from EmilyBonar and NicholasBlaskey April 12, 2024 14:48

AmanuelAaron marked this pull request as ready for review April 12, 2024 14:49

AmanuelAaron requested a review from a team as a code owner April 12, 2024 14:49

NicholasBlaskey reviewed Apr 12, 2024

View reviewed changes

master/internal/api_project.go Outdated Show resolved Hide resolved

add performance tests

04eebc0

AmanuelAaron requested a review from a team as a code owner April 15, 2024 18:46

AmanuelAaron requested a review from NicholasBlaskey April 15, 2024 19:34

AmanuelAaron added 3 commits April 16, 2024 10:38

simplify query

5f6fab3

distinct summary metrics

8cc010f

distinct hyperparams

8cf29b2

EmilyBonar previously requested changes Apr 17, 2024

View reviewed changes

master/internal/api_project.go Outdated Show resolved Hide resolved

master/internal/api_project.go Outdated Show resolved Hide resolved

master/internal/api_project.go Outdated Show resolved Hide resolved

master/internal/api_project.go Show resolved Hide resolved

AmanuelAaron added 5 commits April 17, 2024 20:02

take out null

dcbe11d

small name fix

7690baa

Merge branch 'main' into project-columns

d856955

small update

f64c2c6

filter out string null

1f3678b

AmanuelAaron added 3 commits April 22, 2024 16:38

perf

48d6d92

2nd level nested

f3cd1bf

lint

c0b9249

AmanuelAaron requested a review from johnkim-det April 24, 2024 17:43

AmanuelAaron added 8 commits May 6, 2024 10:42

remove limit change indexes

cc21d07

add project hparam table

0cbb87c

fixes

9b7f3e9

project params logic for move and delete

b2ec65a

lint migrations

4fe2e15

add tests and transaction

996295d

add to exp move

6900e21

Merge branch 'main' into project-columns

56262ac

NicholasBlaskey reviewed May 13, 2024

View reviewed changes

AmanuelAaron and others added 9 commits May 14, 2024 10:30

Update master/internal/api_project.go

cac6506

Co-authored-by: Nick Blaskey <nick.blaskey@hpe.com>

run in tx

c0f9f8a

Merge branch 'project-columns' of github.com:determined-ai/determined…

73ca8f1

… into project-columns

fixes

7e62d35

combine build function into one

89612df

helper function

294d6a1

recursive migration

7409561

update migrations

2d25e82

check for returned runs

805f2f4

AmanuelAaron requested review from johnkim-det and EmilyBonar May 20, 2024 15:50

EmilyBonar approved these changes May 20, 2024

View reviewed changes

NicholasBlaskey approved these changes May 20, 2024

View reviewed changes

AmanuelAaron added 6 commits May 20, 2024 17:08

fixes

f41fea9

recursive project hparam insert

aadc32e

Merge branch 'main' into project-columns

8a6482b

Merge branch 'main' into project-columns

269a320

Merge branch 'main' into project-columns

ad7c753

remove name col

985eb54

AmanuelAaron merged commit 657286c into main May 22, 2024
82 of 98 checks passed

AmanuelAaron deleted the project-columns branch May 22, 2024 20:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Run columns to GetProjectColumns #9146

feat: Add Run columns to GetProjectColumns #9146

AmanuelAaron commented Apr 10, 2024 •

edited

Loading

netlify bot commented Apr 10, 2024 •

edited

Loading

codecov bot commented Apr 10, 2024 •

edited

Loading

EmilyBonar left a comment

NicholasBlaskey May 13, 2024

AmanuelAaron May 14, 2024

NicholasBlaskey May 20, 2024

AmanuelAaron May 20, 2024

EmilyBonar left a comment

NicholasBlaskey left a comment

NicholasBlaskey May 20, 2024

AmanuelAaron May 22, 2024

NicholasBlaskey May 20, 2024

NicholasBlaskey May 20, 2024

AmanuelAaron May 20, 2024

feat: Add Run columns to GetProjectColumns #9146

feat: Add Run columns to GetProjectColumns #9146

Conversation

AmanuelAaron commented Apr 10, 2024 • edited Loading

Description

Test Plan

Checklist

Ticket

netlify bot commented Apr 10, 2024 • edited Loading

✅ Deploy Preview for determined-ui canceled.

codecov bot commented Apr 10, 2024 • edited Loading

Codecov Report

EmilyBonar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmilyBonar left a comment

Choose a reason for hiding this comment

NicholasBlaskey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmanuelAaron commented Apr 10, 2024 •

edited

Loading

netlify bot commented Apr 10, 2024 •

edited

Loading

codecov bot commented Apr 10, 2024 •

edited

Loading