-
Notifications
You must be signed in to change notification settings - Fork 60
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
Feature: Run evaluations in batch #154
Conversation
06b22b1
to
36f3ab5
Compare
packages/jobs/src/job-definitions/batchEvaluations/runBatchEvaluationJob.ts
Outdated
Show resolved
Hide resolved
packages/jobs/src/job-definitions/batchEvaluations/runDocumentJob.ts
Outdated
Show resolved
Hide resolved
36f3ab5
to
0f5b6b4
Compare
10192db
to
73ac5bf
Compare
import { DatasetsRepository } from '@latitude-data/core/repositories' | ||
import { previewDataset } from '@latitude-data/core/services/datasets/preview' | ||
import disk from '$/lib/disk' |
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.
What's the reason to do this?. For using in jobs? I think if you manage to make it work is fine. But also fine that each app init their own disk singleton
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.
we need it in jobs yes
4b7c2b5
to
54de4ca
Compare
runCount: z.number(), | ||
offset: z.number().optional().default(0), | ||
parameters: z.record(z.number()).optional(), | ||
evaluationId: z.number(), |
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.
Shouldn't be this an array?
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.
yes
54de4ca
to
3ce881d
Compare
documentUuid: z.string(), | ||
commitUuid: z.string(), | ||
runCount: z.number(), | ||
offset: z.number().optional().default(0), |
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.
Also limit no?
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.
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.
yes
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.
it's gonna be fromLine and toLine btw
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.
And also a boolean to pick all the rows
better to handle this in frontend, don't send the limits to the backend if the users wants everything
22f0958
to
e696e1e
Compare
commitUuid: z.string(), | ||
runCount: z.number(), | ||
offset: z.number().optional().default(0), | ||
parameters: z.record(z.number()).optional(), |
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.
Why is this a record of numbers?
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.
it maps parameters to the index column of rows in a csv, are you doing it differently in frontend?
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.
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.
yes the key is the parameter name and the value is the column index
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.
so in this case let's assume the csv has last_name and name in columns 3 and 4, the parameters map would be
{ last_name: 3, name: 4} the keys being the document parameters and the values being the csv collumn indeces
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.
It's true, headers is an array 👍
68e20e4
to
3cc826e
Compare
3cc826e
to
f43b471
Compare
This commit implements evaluation runs in batch.