Skip to content
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

Remove metric querying support #1019

Merged
merged 27 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7bcddd5
refactoring integration tests for search_metrics
Mar 13, 2024
e66a915
renaming integration tests
Mar 13, 2024
df2e4a4
wip removing .metric support
Mar 13, 2024
3755278
adapt tests
Mar 15, 2024
78600c9
change metric-context format for searchmetric endpoint
Mar 15, 2024
e31d594
removing support of metric.
Mar 15, 2024
21b2235
set FML_RUN_ORIGINAL_AIM_SERVICE to false
Mar 15, 2024
1681884
adapt query_test
Mar 18, 2024
fc71262
enable aim2 tests
Mar 18, 2024
e3c1606
run TestSearchMetrics only against aim2 package
Mar 18, 2024
126092b
run TestSearchMetrics only against aim2 package
Mar 18, 2024
55e84e8
Merge branch 'main' into search-metric
fabiovincenzi Mar 18, 2024
9322fc5
adapt flows metric_test
Mar 18, 2024
c2303e3
Merge branch 'search-metric' of https://github.com/fabiovincenzi/fast…
Mar 18, 2024
84495d4
run flow tests only over aim2
Mar 18, 2024
3caa110
change query
Mar 19, 2024
d6fe608
restore where clause
Mar 19, 2024
a9b3d31
split psql and sql query building
Mar 20, 2024
0890b6e
Merge branch 'main' into search-metric
fabiovincenzi Mar 20, 2024
9e15649
using struct for metric context tuple
Mar 20, 2024
47b6d92
Merge branch 'search-metric' of https://github.com/fabiovincenzi/fast…
Mar 20, 2024
dab905a
adapt flow tests
Mar 20, 2024
c0247ca
reuse comparejson function
Mar 20, 2024
6ec8b41
Merge branch 'main' into search-metric
fabiovincenzi Mar 20, 2024
c67e081
extract function for findContextIDs
Mar 21, 2024
00a8158
git Merge branch 'search-metric' of https://github.com/fabiovincenzi/…
Mar 21, 2024
5510c3a
Merge branch 'main' into search-metric
fabiovincenzi Mar 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions pkg/api/aim2/api/request/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,20 @@ type SearchRunsRequest struct {
ExcludeTraces bool `query:"exclude_traces"`
}

// MetricTuple represents a metric with key and context.
type MetricTuple struct {
Key string `query:"metric"`
Context string `query:"context"`
}

// SearchMetricsRequest is a request struct for `GET /runs/search/metric` endpoint.
type SearchMetricsRequest struct {
BaseSearchRequest
Query string `query:"q"`
Steps int `query:"p"`
XAxis string `query:"x_axis"`
SkipSystem bool `query:"skip_system"`
Metrics []MetricTuple `query:"m"`
Query string `query:"q"`
Steps int `query:"p"`
XAxis string `query:"x_axis"`
SkipSystem bool `query:"skip_system"`
}

// SearchAlignedMetricsRequest is a request struct for `GET /runs/search/metric/align` endpoint.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package run
package common

import (
"encoding/json"
Expand Down
101 changes: 0 additions & 101 deletions pkg/api/aim2/controller/runs_test.go

This file was deleted.

81 changes: 62 additions & 19 deletions pkg/api/aim2/dao/repositories/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ import (
"context"
"database/sql"
"fmt"
"strings"

"github.com/gofiber/fiber/v2"
"github.com/rotisserie/eris"
"gorm.io/gorm"

"github.com/G-Research/fasttrackml/pkg/api/aim2/api/request"
"github.com/G-Research/fasttrackml/pkg/api/aim2/common"
"github.com/G-Research/fasttrackml/pkg/api/aim2/dao/models"
"github.com/G-Research/fasttrackml/pkg/api/aim2/query"
"github.com/G-Research/fasttrackml/pkg/common/db/types"
Expand Down Expand Up @@ -103,7 +105,7 @@ func (r MetricRepository) SearchMetrics(
return nil, 0, nil, err
}

if !pq.IsMetricSelected() {
if req.Metrics == nil || len(req.Metrics) == 0 {
return nil, 0, nil, eris.New("No metrics are selected")
}

Expand Down Expand Up @@ -169,30 +171,45 @@ func (r MetricRepository) SearchMetrics(
result[r.ID] = SearchResult{int64(r.RowNum), run}
}

ids, err := r.findContextIDs(ctx, &req)
if err != nil {
return nil, 0, nil, eris.Wrap(err, "error finding context ids")
}

var metricKeyContextConditionSlice []string
for i, tuple := range req.Metrics {
condition := fmt.Sprintf("(latest_metrics.key = '%s' AND contexts.id = %d)", tuple.Key, ids[i])
metricKeyContextConditionSlice = append(metricKeyContextConditionSlice, condition)
}
metricKeyContextCondition := strings.Join(metricKeyContextConditionSlice, " OR ")

subQuery := r.db.WithContext(ctx).
Select(
"runs.run_uuid",
"runs.row_num",
"latest_metrics.key",
"latest_metrics.context_id",
"contexts.json AS context_json",
fmt.Sprintf("(latest_metrics.last_iter + 1)/ %f AS interval", float32(req.Steps)),
).
Table("runs").
Joins(
"INNER JOIN experiments ON experiments.experiment_id = runs.experiment_id AND experiments.namespace_id = ?",
namespaceID,
).
Joins("LEFT JOIN latest_metrics USING(run_uuid)").
Joins("LEFT JOIN contexts ON latest_metrics.context_id = contexts.id").
Where(metricKeyContextCondition)

tx := r.db.WithContext(ctx).
Select(`
metrics.*,
runmetrics.context_json`,
metrics.*,
runmetrics.context_json`,
).
Table("metrics").
Joins(
"INNER JOIN (?) runmetrics USING(run_uuid, key, context_id)",
pq.Filter(r.db.WithContext(ctx).
Select(
"runs.run_uuid",
"runs.row_num",
"latest_metrics.key",
"latest_metrics.context_id",
"contexts.json AS context_json",
fmt.Sprintf("(latest_metrics.last_iter + 1)/ %f AS interval", float32(req.Steps)),
).
Table("runs").
Joins(
"INNER JOIN experiments ON experiments.experiment_id = runs.experiment_id AND experiments.namespace_id = ?",
namespaceID,
).
Joins("LEFT JOIN latest_metrics USING(run_uuid)").
Joins("LEFT JOIN contexts ON latest_metrics.context_id = contexts.id")),
pq.Filter(subQuery),
).
Where("MOD(metrics.iter + 1 + runmetrics.interval / 2, runmetrics.interval) < 1").
Order("runmetrics.row_num DESC").
Expand Down Expand Up @@ -234,3 +251,29 @@ func (r MetricRepository) GetContextListByContextObjects(
}
return contexts, nil
}

func (r MetricRepository) findContextIDs(ctx context.Context, req *request.SearchMetricsRequest) ([]uint, error) {
contextList := []types.JSONB{}
contextsMap := map[string]types.JSONB{}
for _, r := range req.Metrics {
data := types.JSONB(r.Context)
contextList = append(contextList, data)
contextsMap[string(data)] = data
}

contexts, err := r.GetContextListByContextObjects(ctx, contextsMap)
if err != nil {
return nil, fmt.Errorf("error getting context list: %w", err)
}

ids := make([]uint, len(contextList))
for _, context := range contexts {
for i := 0; i < len(contextList); i++ {
if common.CompareJson(contextList[i], context.Json) {
ids[i] = context.ID
}
}
}

return ids, nil
}
50 changes: 0 additions & 50 deletions pkg/api/aim2/query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ type QueryParser struct {

type ParsedQuery interface {
Filter(*gorm.DB) *gorm.DB
IsMetricSelected() bool
}

type parsedQuery struct {
Expand Down Expand Up @@ -187,10 +186,6 @@ func (pq *parsedQuery) Filter(tx *gorm.DB) *gorm.DB {
return tx
}

func (pq *parsedQuery) IsMetricSelected() bool {
return pq.metricSelected
}

func (pq *parsedQuery) parseNode(node ast.Expr) (any, error) {
ret, err := pq._parseNode(node)
if err != nil && !errors.Is(err, SyntaxError{}) {
Expand Down Expand Up @@ -644,51 +639,6 @@ func (pq *parsedQuery) parseName(node *ast.Name) (any, error) {
}
},
), nil
case "metric":
table, ok := pq.qp.Tables["metrics"]
if !ok {
return nil, errors.New("unsupported name identifier 'metric'")
}
return attributeGetter(
func(attr string) (any, error) {
switch attr {
case "name":
pq.metricSelected = true
return clause.Column{
Table: table,
Name: "key",
}, nil
case "last":
return clause.Column{
Table: table,
Name: "value",
}, nil
case "last_step":
return clause.Column{
Table: table,
Name: "last_iter",
}, nil
case "first_step":
return 0, nil
case "context":
return attributeGetter(
func(contextKey string) (any, error) {
// Add a WHERE clause for the context key
return Json{
Column: clause.Column{
Table: TableContexts,
Name: "json",
},
JsonPath: contextKey,
Dialector: pq.qp.Dialector,
}, nil
},
), nil
default:
return nil, fmt.Errorf("unsupported metrics attribute %q", attr)
}
},
), nil
case "re":
return attributeGetter(
func(attr string) (any, error) {
Expand Down
Loading