Sarc 229 - Add function load_job_series()
#89
Conversation
Add count_jobs() to count jobs. Add _compute_jobs_query() to compute MongoDB query, and use it in count_jobs() and get_jobs().
… end of for loop.
…series(). Display time-related fields in tests. Mock datetime.now() so that file regressions can find same output in tests.
sarc/jobs/series.py
Outdated
| job_series["cpu"] = ( | ||
| max(billing, job.allocated.cpu) if job.allocated.cpu else 0 | ||
| ) | ||
| job_series["gpu_requested"] = gres_gpu |
There was a problem hiding this comment.
Looks like there is a bug here (probably coming from my initial example, sorry!). It should be job.requested.gres_gpu.
There was a problem hiding this comment.
Should I change billing too ? https://github.com/mila-iqia/SARC/blob/sarc-229/sarc/jobs/series.py#L310
billing = job.allocated.billing or 0
gres_gpu = job.allocated.gres_gpu or 0
There was a problem hiding this comment.
No, for billing it's the allocated values. That's good.
There was a problem hiding this comment.
OK ! Done: 6e0d8f2
There was a problem hiding this comment.
Oups, je voulais dire que c'était correct, faut laisser comme c'était. 😅
There was a problem hiding this comment.
Actuellement, c'est comme ceci;
billing = job.allocated.billing or 0
gres_gpu = job.requested.gres_gpu or 0
Que faut-il changer ?
There was a problem hiding this comment.
Mise à jour dans le dernier commit: 2f60e3a
Co-authored-by: Xavier Bouthillier <[email protected]>
Co-authored-by: Xavier Bouthillier <[email protected]>
…ce over job fields.
sarc/jobs/series.py
Outdated
| max(billing, job.allocated.cpu) if job.allocated.cpu else 0 | ||
| ) | ||
| job_series["gpu_requested"] = gres_gpu | ||
| job_series["duration"] = job.elapsed_time |
There was a problem hiding this comment.
This also is not necessary in the end. Leftover of my exemple, sorry. 😅
There was a problem hiding this comment.
Done, mem also removed: bd317ed
NB: About mem, it was copied from job.allocated.mem. I just remarked that job.allocated is copied as a dict inside data frame, so someone who wants to just get the mem value will have to do something like frame['allocated'][row]['mem']. It is ok ?
There was a problem hiding this comment.
Non en effet, je viens de réaliser ça: bd317ed.
There was a problem hiding this comment.
Et dans le fond, quand on aura allocated et requested en versions flattened, on aura les champs {requested,allocated}_gpu.
Ce serait peut-être bien dans ce cas qu'on utilise le même format, c'est à dire requested_gpu par exemple. C'est quand même utile quand fasse l'opération qu'on fait là, sinon billing est difficile à utiliser dans un dataframe. La raison pour laquelle on a le if-else, c'est que billing s'applique à allocated_gpu si c'est une job avec GPU, sinon c'est à allocated_cpu.
There was a problem hiding this comment.
Mis à jour dans le dernier commit! 2f60e3a
Rename column cpu -> cpu_allocated Add column cpu_requested
| @pytest.mark.usefixtures("read_only_db", "tzlocal_is_mtl") | ||
| @pytest.mark.parametrize("params", few_parameters.values(), ids=few_parameters.keys()) | ||
| def test_load_job_series_fields_list(params, file_regression): | ||
| fields = ["gpu_memory", "user", "work_dir"] |
There was a problem hiding this comment.
Faudrait garder la mem mais avec le nouveau nom. D'après la liste ALL_COLUMNS je crois comprendre que allocated et requested ne sont pas flattened comme je l'aurais pensé. Faudrait par exemple que les attributes de allocated apparaissent tel que allocated.<nomdelattribut>.
There was a problem hiding this comment.
Ok ! Fait dans le dernier commit: 2f60e3a
Co-authored-by: Xavier Bouthillier <[email protected]>
…all potential fields.
Hello @bouthilx @nurbal ! This is a PR for SARC-229 !