Allow run_uid#1405
Conversation
CLI accepts --run-uid as an argument and will use that for the run_uid of the benchmark run instead of an automatically generated run_uid.
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
| run_uid: str | None, | ||
| ): | ||
| _run_uid = run_uid if run_uid else f"run-{benchmark.uid}-{start_time.strftime('%Y%m%d-%H%M%S')}" | ||
| with open(json_path, "w") as f: |
There was a problem hiding this comment.
I think before it was unlikely due to the timestamp, but now there's potential that json_path already exists if someone re-uses a run_uid, right? Should we check / fail out in that case?
There was a problem hiding this comment.
I'm fine either way, but I'm also not sure I'd consider it the responsibility of modelbench to ensure that the optional run-uid argument is used responsibly.
Where would you expect that check to happen? In the cli before anything is run?
There was a problem hiding this comment.
Yah, that's fair, though maybe just here, it could warn if overwriting? I don't feel particularly strongly about this though!
There was a problem hiding this comment.
The reason I'd be reluctant to put it here specifically is because this is after the benchmark is run, so it would kind of be awful to run a long benchmark and then puke if the path already exists.
Maybe that would be reasonable because things should be cached and a re-run would be fast? Unless the cache also uses some part of the run uid. Hmm.
Let's talk about it in standup.
CLI accepts --run-uid as an argument and will use that for the run_uid of the benchmark run instead of an automatically generated run_uid.