Add inference to runner #186
Conversation
| @dataclass(frozen=True) | ||
| class RunRecovery: | ||
| """ |
There was a problem hiding this comment.
Which of the bits are really essential? Can we simplify? At the moment, it seems to be a glorified list, even though we use at most 1 checkpoint, or?
There was a problem hiding this comment.
@ant0nsc - can I propose to look into this and the CheckpointHandler today, but make changes in a separate PR when I am back?
| for p in self.checkpoints_roots: | ||
| logging.info(str(p)) | ||
|
|
||
|
|
||
| class CheckpointHandler: |
There was a problem hiding this comment.
The dreaded checkpointhandler... (or at least I dread it). which parts of that could we strip out? I never fully got the purpose of this class.
| @@ -11,46 +11,31 @@ dependencies: | |||
| - python-blosc==1.7.0 | |||
| - torchvision=0.11.1 | |||
| - pip: | |||
| - -r ../test_requirements.txt | |||
There was a problem hiding this comment.
shouldn't be the other way round? I think test_requirements should include all the requirements of the package plus optionally other packages required for testing.
I think a developer will naturally update the package environment if something new is needed while developing and the current setup will not ensure the tests environment will also be updated
There was a problem hiding this comment.
or maybe I don't understand correctly what test_requirements.txt is used for? if it's more of a generic hi-ml package requirement file maybe we can rename it?
Add inference to the hi-ml runner.
Also in this PR: lightning-bolts is downgraded to 0.4.0 in line with InnerEye, since we are not ready to move over yet
Please follow the guidelines for PRs contained here. Checklist:
make htmlin the `docs folder)Added/Changed/Removed/... in the "Upcoming" section.
and if needed a motivation why that change was required.