Add ability to mark jobs as discarded and also add scoring#1965
Conversation
✅ Deploy Preview for transformerlab canceled.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| logger.debug("Trackio integration failed during finish()", exc_info=True) | ||
| if score is not None: | ||
| _run_async(self._job.update_job_data_field("score", score)) # type: ignore[union-attr] | ||
| resolved_score: Dict[str, Any] = {} |
There was a problem hiding this comment.
I think this is OK but just double checking:
We don't set score anywhere earlier? I just noticed that if you call finish without a score I think it will overwrite if there's anythign in there already. But I think that's fine?
There was a problem hiding this comment.
No score is only set by finish, nowhere before that
| filtered = {k: v for k, v in updates.items() if k in allowed_keys} | ||
|
|
||
| # Keep discard under job_data.score.discard to avoid introducing a new top-level field. | ||
| if "discard" in filtered: |
There was a problem hiding this comment.
Technically this is race. Not sure how important (not super worried about two people flipping discard on same job at the same time, but maybe UI and SDK fighting is possible).
OK I asked Claude for a simple fix and it said to create a function at service later and call from here if "discard" is passed. Something like this (note this is technically still a race, just narrower and cleaner):
async def job_update_job_data_score_field(job_id, score_key: str, value, experiment_id):
"""Merge a single key into job_data.score atomically."""
resolved_id = await _resolve_full_job_id(str(job_id), str(experiment_id))
actual_id = resolved_id or str(job_id)
job = await Job.get(actual_id, experiment_id)
# Read + merge + write within the same Job instance scope
current = (await job.get_job_data()) or {}
score = current.get("score") if isinstance(current, dict) else {}
score = dict(score) if isinstance(score, dict) else {}
score[score_key] = value
await job.update_job_data_field("score", score)
await cache.invalidate(f"job:{actual_id}")
await cache.delete(_job_cache_key(actual_id))
There was a problem hiding this comment.
Fixed now based on what you recommended
|
|
||
| # Keep discard under job_data.score.discard to avoid introducing a new top-level field. | ||
| if "discard" in filtered: | ||
| discard_value = bool(filtered.pop("discard")) |
There was a problem hiding this comment.
Probably fine but as a note "false" will evaluate as true I think.
There was a problem hiding this comment.
Okay fixed this and also found similar things in CLI and frontend, fixed both
deep1401
changed the title
There was a problem hiding this comment.
Improved the existing jobs filter by score by allowing asc and desc order and also placing other existing jobs at the end:
# Default descending by metric:
lab job list --score-metric eval/loss
# Explicit ascending:
lab job list --score-metric eval/loss --score-order asc
3 participants
GUI: added a new dropdown action in the jobs list menu:
CLI: added
lab job discard <job_id>andlab job discard <job_id> --undoto toggle score.discard.Also improved the existing jobs filter by score by allowing asc and desc order and also placing other existing jobs at the end: