-
Notifications
You must be signed in to change notification settings - Fork 35
Add status and public/secret scores to /user/submissions list #502
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1257,7 +1257,10 @@ def get_user_submissions( | |
| offset: Offset for pagination | ||
|
|
||
| Returns: | ||
| List of submission dictionaries with summary info and runs | ||
| List of submission dictionaries with summary info and runs. Each | ||
| entry includes ``status`` ("pending"/"failed"/"done"), | ||
| ``public_score`` and ``secret_score`` (the geomean leaderboard | ||
| scores, either may be ``None``), plus the public ``runs`` list. | ||
| """ | ||
| # Validate and clamp inputs | ||
| limit = max(1, min(limit, 100)) | ||
|
|
@@ -1325,17 +1328,59 @@ def get_user_submissions( | |
| "score": run_row[2], | ||
| }) | ||
|
|
||
| # Per-submission status and leaderboard scores. The public `runs` | ||
| # above are ranking-filtered (anti-cheat: the public score is hidden | ||
| # unless the matching secret run passed). Here we additionally | ||
| # surface the secret leaderboard score (visible to the owner, as the | ||
| # detail endpoint already does) and whether any run failed, so | ||
| # callers can show an accurate status and both scores without an | ||
| # extra request per submission. | ||
| agg_query = """ | ||
| SELECT submission_id, | ||
| MIN(score) FILTER ( | ||
| WHERE mode = 'leaderboard' AND secret AND passed | ||
| ) AS secret_score, | ||
| bool_or(NOT passed) AS has_failed_run | ||
| FROM leaderboard.runs | ||
| WHERE submission_id = ANY(%s) | ||
| GROUP BY submission_id | ||
|
Comment on lines
+1348
to
+1355
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you benchmark how long this sequel query will take?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. On the docker-compose test Postgres, seeded with 100 submissions × 6 runs (600 rows), the added aggregate query runs at ~0.25 ms/call for a full 100-submission page, versus ~21 ms for the whole |
||
| """ | ||
| self.cursor.execute(agg_query, (submission_ids,)) | ||
| agg_by_submission: dict = { | ||
| row[0]: {"secret_score": row[1], "has_failed_run": row[2]} | ||
| for row in self.cursor.fetchall() | ||
| } | ||
|
|
||
| # Build result with runs grouped by submission | ||
| results = [] | ||
| for row in submissions: | ||
| sub_id = row[0] | ||
| done = row[4] | ||
| public_runs = runs_by_submission.get(sub_id, []) | ||
| agg = agg_by_submission.get(sub_id, {}) | ||
|
|
||
| # The public leaderboard score (lowest across GPUs), already | ||
| # ranking-eligible by construction of `runs_query`. | ||
| public_scores = [r["score"] for r in public_runs if r["score"] is not None] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused. I thought the public scores were already reported by the submissions endpoint. So why do we need new code to report it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right — the public score was already exposed (in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol bro come on I expected better.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that one's on me — I added a field that already existed in |
||
| public_score = min(public_scores) if public_scores else None | ||
|
|
||
| if not done: | ||
| status = "pending" | ||
| elif agg.get("has_failed_run"): | ||
| status = "failed" | ||
| else: | ||
| status = "done" | ||
|
|
||
| results.append({ | ||
| "id": sub_id, | ||
| "leaderboard_name": row[1], | ||
| "file_name": row[2], | ||
| "submission_time": row[3], | ||
| "done": row[4], | ||
| "runs": runs_by_submission.get(sub_id, []), | ||
| "done": done, | ||
| "status": status, | ||
| "public_score": public_score, | ||
| "secret_score": agg.get("secret_score"), | ||
| "runs": public_runs, | ||
| }) | ||
| return results | ||
| except psycopg2.Error as e: | ||
|
|
||
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.
I don't understand this here. Can you explain this to me? Why
MIN(score)?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.
A submission can have a secret
leaderboardrun per GPU type, so there can be more than one secret score.MINtakes the best (lowest = fastest) one, matching how the existing public score is summarized for the row (therunsare likewise per-GPU and the caller/CLI takes the min). For single-GPU leaderboards like qr_v2 there's exactly one, so MIN is just that value. Happy to switch to per-GPU secret scores instead if you'd prefer symmetry withruns, but a single ranking number seemed more useful for the list view.