-
Notifications
You must be signed in to change notification settings - Fork 58
Link ComponentJobUtilization and Invoice
#4718
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 all commits
42bfa7e
55df239
da01369
e1cff9f
dbdf0b9
481410b
a3e05c2
e30357f
4fd8c03
a1f804c
00f2518
52a9119
eb4fcc8
67b7e65
ea2ee18
aeded35
8a0e697
e9cdee8
28dbd6c
46bc8e5
717ed43
616630d
86a7c3b
3dc5607
2d716c0
100bcb6
26dbb58
b05492a
d702e8a
7a12dbf
0dcbab3
0db089e
34af345
036ecd2
a9935ae
dab2b7f
3149caf
378895e
1ffcf56
ada1dfa
738f5e1
aae7895
ea9c4f9
77601c6
10a52df
f3c78fd
734e091
2e74309
5b63538
1fd0a1f
b2aa2e9
6d85474
bec29cf
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 |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| from django.utils import timezone | ||
|
|
||
| from grandchallenge.algorithms.exceptions import TooManyJobsScheduled | ||
| from grandchallenge.challenges.exceptions import InsufficientBudgetError | ||
| from grandchallenge.components.schemas import GPUTypeChoices | ||
| from grandchallenge.components.tasks import ( | ||
| provision_invocation_input_data, | ||
|
|
@@ -165,6 +166,21 @@ def create_algorithm_jobs( | |
| input_civ_set=ai.values.all(), | ||
| use_warm_pool=use_warm_pool, | ||
| ) | ||
| jobs.append( | ||
| job | ||
| ) # Keep track of created jobs, even if utilization setup has failed | ||
|
|
||
| if job_utilization_challenge is not None: | ||
|
chrisvanrun marked this conversation as resolved.
|
||
| try: | ||
| job.utilization.invoice = ( | ||
| job_utilization_challenge.active_invoice | ||
| ) | ||
| except InsufficientBudgetError: | ||
| job.update_status( | ||
| status=Job.FAILURE, | ||
| error_message="Job cannot be executed. The challenge has insufficient budget to run this job.", | ||
| ) | ||
| break | ||
|
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. With James' comment here, this will need to be changed. The invoice will need to be passed along as a kwarg to this function. |
||
|
|
||
| job.utilization.archive = ai.archive | ||
| job.utilization.phase = job_utilization_phase | ||
|
|
@@ -173,8 +189,6 @@ def create_algorithm_jobs( | |
|
|
||
| job.execute() | ||
|
|
||
| jobs.append(job) | ||
|
|
||
| return jobs | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,19 +20,20 @@ | |
| ) | ||
|
|
||
|
|
||
| def annotate_compute_costs(*, challenge): | ||
| def annotate_compute_costs(*, invoice): | ||
| algorithm_job_utilizations = JobUtilization.objects.filter( | ||
| challenge=challenge | ||
| invoice=invoice, | ||
| ) | ||
| job_warm_pool_utilizations = JobWarmPoolUtilization.objects.filter( | ||
| challenge=challenge | ||
| invoice=invoice, | ||
| ) | ||
| evaluation_job_utilizations = EvaluationUtilization.objects.filter( | ||
| challenge=challenge | ||
| invoice=invoice, | ||
| ) | ||
|
|
||
| update_compute_cost_euro_millicents( | ||
| obj=challenge, | ||
| obj=invoice, | ||
| obj_field="compute_costs_utilized_euros_millicents", | ||
| algorithm_job_utilizations=algorithm_job_utilizations, | ||
| job_warm_pool_utilizations=job_warm_pool_utilizations, | ||
| evaluation_job_utilizations=evaluation_job_utilizations, | ||
|
|
@@ -54,6 +55,7 @@ def annotate_job_duration_and_compute_costs(*, phase): | |
|
|
||
| update_compute_cost_euro_millicents( | ||
| obj=phase, | ||
| obj_field="compute_cost_euro_millicents", | ||
| algorithm_job_utilizations=algorithm_job_utilizations, | ||
| job_warm_pool_utilizations=job_warm_pool_utilizations, | ||
| evaluation_job_utilizations=evaluation_job_utilizations, | ||
|
|
@@ -63,6 +65,7 @@ def annotate_job_duration_and_compute_costs(*, phase): | |
| def update_compute_cost_euro_millicents( | ||
| *, | ||
| obj, | ||
| obj_field, | ||
|
Member
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. Why is this new kwarg necessary?
Member
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 see now. The construction indicates a design problem. Please rename This is for several reasons:
Member
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. The name was chosen deliberately to contrast it with the existing
Rather than renaming the
Member
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. That cannot be done without downtime and/or a data migration, so rename the new one. |
||
| algorithm_job_utilizations, | ||
| job_warm_pool_utilizations, | ||
| evaluation_job_utilizations, | ||
|
|
@@ -79,8 +82,10 @@ def update_compute_cost_euro_millicents( | |
|
|
||
| items = [algorithm_job_costs, job_warm_pool_costs, evaluation_costs] | ||
|
|
||
| obj.compute_cost_euro_millicents = sum( | ||
| item["compute_cost_euro_millicents__sum"] or 0 for item in items | ||
| setattr( | ||
| obj, | ||
| obj_field, | ||
| sum(item["compute_cost_euro_millicents__sum"] or 0 for item in items), | ||
| ) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| class InsufficientBudgetError(Exception): | ||
| pass |
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.
The comment might be confusing. I started rewriting it until I realized this comment is not really necessary at all, the code explains itself.
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.
This code expects that the returned jobs are the ones that have been scheduled, so the utilisation set up must not fail.