Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Train overhaul #373

Merged
merged 58 commits into from
Feb 26, 2025
Merged

Train overhaul #373

merged 58 commits into from
Feb 26, 2025

Conversation

pattonw
Copy link
Contributor

@pattonw pattonw commented Feb 18, 2025

Overhaul Trainers.
Removed Trainer class and all subclasses, we now only have TrainerConfig and its subclasses.

breaking api change
Moved much of the functionality (parallelization, snapshot saving, batching etc.) from the TrainerConfig to the RunConfig.
This includes arguments such as num_workers, snapshot_interval, batch_size. The only thing that must be updated is moving these arguments from the TrainerConfig to the RunConfig.

use hidden attributes (with an _) in the attrs classes so we can have abstract properties and allow architectures to take them as input
redefine dims on architectures
We want to make runs more easily usable. Making changes is easier when only one class needs to be modified. Also makes it easier to learn and use the framework.
this allows the modelzoo save to describe the output channels
Make parent weights directory if it doesn't exist.
Save dummy file if tracing fails to prevent excessive trace attempts
Add support for loading scripted models via paths
Add support for freezing layers
unzip first, then open. I think this should be done automatically by bioimageio.core
@mzouink
Copy link
Member

mzouink commented Feb 18, 2025

@pattonw can you please check this bug
the error is coming from funlib.persistence

dacapo/experiments/__init__.py:2: in <module>
    from .run import Run  # noqa
dacapo/experiments/run.py:1: in <module>
    from .run_config import RunConfig
dacapo/experiments/run_config.py:7: in <module>
    from funlib.persistence import prepare_ds, open_ds
/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/funlib/persistence/__init__.py:1: in <module>
    from .arrays import Array, open_ds, prepare_ds, open_ome_ds, prepare_ome_ds  # noqa
/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/funlib/persistence/arrays/__init__.py:3: in <module>
    from .ome_datasets import prepare_ome_ds, open_ome_ds  # noqa
/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/funlib/persistence/arrays/ome_datasets.py:5: in <module>
    from iohub.ngff import AxisMeta, TransformationMeta, open_ome_zarr
E   ImportError: cannot import name 'AxisMeta' from 'iohub.ngff' (/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/iohub/ngff/__init__.py)

@pattonw
Copy link
Contributor Author

pattonw commented Feb 18, 2025

Oh I see what happened. I fixed them locally and thought the fix would get pushed, but this is a bug in funlib.persistence ngff branch.
I'll fix it today

@mzouink
Copy link
Member

mzouink commented Feb 19, 2025

tests are successfull. @pattonw do i merge ?

@pattonw
Copy link
Contributor Author

pattonw commented Feb 19, 2025

Black is just failing due to not being able to modify the pull request since it's coming from my fork
Mypy is still failing with quite a few errors. I'll fix those soon but that can be a separate pull request if you want, it shouldn't affect usability

@mzouink mzouink merged commit 380fbb3 into janelia-cellmap:main Feb 26, 2025
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants