Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@Dando18 will you be able to add some tests here? |
ZwFink
left a comment
ZwFink
left a comment
There was a problem hiding this comment.
Is it intended that IR can be set on jobs that are in the queue at the time of the call?
|
@ZwFink good point, this is probably worth changing. I had previously only been using set_ir before submitting any work, but it's probably good to make the async behavior better. I could either make it an error to set_ir while the queue is non-empty or I could add a new Future type to the queue for processing IR changes. |
ZwFink
left a comment
ZwFink
left a comment
There was a problem hiding this comment.
This LGTM. Is it a problem that IR revisions aren't persisted to ExperimentResult?
| future = EvalFuture(job_id, config) | ||
|
|
||
| future = EvalFuture(job_id, config, self._ir_revision, self._ir_data) | ||
| self._futures[job_id] = future |
There was a problem hiding this comment.
It's worth investigating at some point whether completed futures should be pruned, now that they may have large payload. Though we expect (I think?) set_ir calls to be infrequent
|
☝️ Rebase onto main |
3 participants
Closes #122