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

Re-enable experiment endpoint #377

Merged
merged 18 commits into from
Nov 21, 2024

Conversation

javiermtorres
Copy link
Contributor

What's changing

An experiment endpoint reusing the jobs service is made available.

How to test it

N/A for the moment

Additional notes for reviewers

N/A for the moment

I already...

  • added some tests for any new functionality
  • updated the documentation
  • checked if a (backend) DB migration step was required and included it if required

@javiermtorres javiermtorres force-pushed the 340-experiment-workflow-for-eval-only branch from 0b8a657 to 2f4c347 Compare November 17, 2024 11:46
@javiermtorres javiermtorres force-pushed the 340-experiment-workflow-for-eval-only branch from 89d5545 to 9628c8b Compare November 17, 2024 19:15
Copy link
Contributor

@ividal ividal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! At least the UI will not have to worry about what we decide regarding tracking, workflows, etc. :)

Tested it as well (upload dialogsum, launch experiment based on the included ground-truth), listed experiments, downloaded experiment results.

@ividal
Copy link
Contributor

ividal commented Nov 17, 2024

By the way, we're going to need to introduce some tests for this one - current tests (not even integration ones) do not cover this functionality.

@javiermtorres
Copy link
Contributor Author

By the way, we're going to need to introduce some tests for this one - current tests (not even integration ones) do not cover this functionality.

Roger and wilco :)

@javiermtorres javiermtorres marked this pull request as ready for review November 18, 2024 13:04
@veekaybee
Copy link
Member

I'd like to better understand the use-case for this. Generally, it would make sense to write a separate experiment service and we decide on how we'd like tracking to work at the experiment level. I'd be hesitant at this stage to mix jobs and experiments since they're different levels of hierarchy at the moment.

@ividal
Copy link
Contributor

ividal commented Nov 18, 2024

I'd like to better understand the use-case for this. Generally, it would make sense to write a separate experiment service and we decide on how we'd like tracking to work at the experiment level. I'd be hesitant at this stage to mix jobs and experiments since they're different levels of hierarchy at the moment.

The goal is to abstract API consumers from the changes around /experiments and `/jobs, and avoid breaking the API unless we are permanently removing functionality on purpose).

I completely agree with you that jobs are a different abstraction, which is why /experiments/evaluation and experiments/inference were not something that we intended folks to use, they were a temporary by-product.

Here, the functionality folks could rely on was "/experiments carries out some steps to let me specify some data, a model and get a score back", whereas jobs aims at giving the person full control of one of the steps. We can keep that spirit (and in fact it's what we want in the future, when tracking is integrated), so we don't need to have a dummy /experiments around.

In the near future, we'll have to decide how exactly experiments will collect/trigger/x multiple jobs and how we track them. Until then, the UI doesn't need to care and can still use them as an entrypoint to workflows.

Copy link
Contributor

@ividal ividal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just a tiny thing there and I'd merge. We're far from sorting out clean jobs, experiments and tracking here, but this is a small step to not bothering UI with the debate.

@github-actions github-actions bot added the api Changes which impact API/presentation layer label Nov 20, 2024
@javiermtorres javiermtorres merged commit 000dafb into main Nov 21, 2024
7 checks passed
@javiermtorres javiermtorres deleted the 340-experiment-workflow-for-eval-only branch November 21, 2024 11:54
@ividal ividal changed the title Enable dummy experiment endpoint Re-enable experiment endpoint Dec 9, 2024
@ividal ividal mentioned this pull request Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Changes which impact API/presentation layer backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Experiment workflow for eval-only (i.e. when dataset already has ground-truth)
3 participants