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

Clean MapsManager #658

Merged

Conversation

camillebrianceau
Copy link
Collaborator

@camillebrianceau camillebrianceau commented Sep 27, 2024

This PR refactors the training methods in the trainer module.

  • First, I removed the _train_multi method. Now, the train_single method is used for both single and multi-network training. In a future PR, we will need to apply the same approach to the _train_ssda method.
  • I also moved the loops for split in split_manager.iterate(): and for network in range(num_network): out of the train_single method. As a result, the new _train method will be the entry point for trainer.train().
  • Additionally, I added an example of the future API in API_test.py.
  • I created a TmpConfig, which is currently unused but retained as a model for future reference.

Next PR:

  • Plan to extract the MapsManager, Validator, and Splitter from the trainer.

@camillebrianceau camillebrianceau changed the base branch from dev to refactoring September 27, 2024 11:10
@pep8speaks
Copy link

pep8speaks commented Sep 27, 2024

Hello @camillebrianceau! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 43:9: E266 too many leading '#' for block comment
Line 70:9: E266 too many leading '#' for block comment

Line 53:1: E266 too many leading '#' for block comment

Comment last updated at 2024-10-01 13:49:26 UTC

@camillebrianceau camillebrianceau marked this pull request as ready for review September 30, 2024 12:39
Copy link
Collaborator

@thibaultdvx thibaultdvx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except suspect prints.

clinicadl/trainer/tasks_utils.py Outdated Show resolved Hide resolved
@camillebrianceau camillebrianceau merged commit bf67c1c into aramis-lab:refactoring Oct 1, 2024
21 checks passed
@camillebrianceau camillebrianceau deleted the cb_init_maps branch October 1, 2024 14:19
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.

3 participants