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

Allow multiple fit runs #170

Closed

Conversation

roman807
Copy link
Collaborator

closes #158

  • records results as json files
  • overwrites Trainer, CSVLogger and TensorBoardLogger to add log_dir-setter
  • adds _adapt_log_dirs method to interface to set the dir for logger, trainer, callback after 'evaluation_id' is created
  • adds loop to interface to run n_runs times

to test, run

eva fit --config configs/vision/tests/patch_camelyon.yaml 

and check the results in logs/dino_vits16/patch_camelyon/<evaluation_id>

@roman807 roman807 linked an issue Feb 26, 2024 that may be closed by this pull request
@roman807 roman807 requested a review from ioangatop February 26, 2024 20:34
@roman807 roman807 marked this pull request as ready for review February 27, 2024 14:20
@roman807 roman807 requested a review from nkaenzig February 27, 2024 14:20
Copy link
Collaborator

@nkaenzig nkaenzig left a comment

Choose a reason for hiding this comment

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

Looks good! I left some initial comments, will continue the review later.

src/eva/interface/interface.py Outdated Show resolved Hide resolved
if data.datasets.test is not None:
trainer.test(datamodule=data)
for run_id in range(n_runs):
trainer_run = copy.deepcopy(trainer)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copying the trainer and model for each run could be resource-intensive, especially for heavy models. Not sure if there is a better way to do this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

haven't seen another safe way yet. Let me know if you have a simple idea, otherwise lets open another issue for it

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe @ioangatop has another idea. elsewise I'd say let's leave it as is.

src/eva/interface/interface.py Outdated Show resolved Hide resolved
src/eva/loggers/csv_logger.py Outdated Show resolved Hide resolved
src/eva/utils/recorder.py Outdated Show resolved Hide resolved
src/eva/utils/recorder.py Outdated Show resolved Hide resolved
@roman807 roman807 linked an issue Feb 28, 2024 that may be closed by this pull request
@ioangatop ioangatop closed this Mar 11, 2024
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.

Allow multiple fit runs and report average and range Record evaluation outputs
3 participants