DeepRegNet / DeepReg

Medical image registration using deep learning
Apache License 2.0
564 stars 76 forks source link

Add support for W&B #709

Closed mathpluscode closed 2 years ago

mathpluscode commented 3 years ago

Subject of the feature

It can be very helpful to allow users to upload their training results to W&B https://wandb.ai/site in addition to tensorboard.

This feature should be activated using either config / env variable.

NMontanaBrown commented 3 years ago

I could do this, I have experience with WandB?

NMontanaBrown commented 3 years ago

TODOs:

NMontanaBrown commented 3 years ago

Not sure how we are going to test that WandB is logging a run though. @mathpluscode @YipengHu any ideas?

mathpluscode commented 3 years ago

Not sure how we are going to test that WandB is logging a run though. @mathpluscode @YipengHu any ideas?

We will need to mock these functionalities.

NMontanaBrown commented 3 years ago

Not sure how we are going to test that WandB is logging a run though. @mathpluscode @YipengHu any ideas?

We will need to mock these functionalities.

How? Any links? Not even sure we can mock this.

mathpluscode commented 3 years ago

Not sure how we are going to test that WandB is logging a run though. @mathpluscode @YipengHu any ideas?

We will need to mock these functionalities.

How? Any links? Not even sure we can mock this.

Here's an example mocking gcloud functions https://stackoverflow.com/questions/62803901/python-unittest-mock-google-storage-how-to-achieve-exceptions-notfound-as-side

The main idea is just to pretend that we've called the wandb functions. The package would be https://docs.python.org/3/library/unittest.mock.html. This is the common choice instead of using pytest.

For further details, I do not understand these 100% so better to google it ;)

NMontanaBrown commented 3 years ago

Maybe cose #476 because this is available in WB.

YipengHu commented 3 years ago

Maybe cose #476 because this is available in WB.

@mathpluscode ?

mathpluscode commented 3 years ago

Maybe cose #476 because this is available in WB.

@NMontanaBrown What WB has are basic methods like grid search or random search or Bayesian. Ofc it's a good starting point, but I think we do not have to close that issue, as:

  1. adding W&B for logging and adding W&B for hyper parameters can be two different things. Specially hyper parameter tuning requires changes of configs I believe.

  2. we may want to look at some more advanced tuning techniques:)

NMontanaBrown commented 3 years ago

Ok, in any case @YipengHu and I talked about closing this ticket soon because WandB should provide tests for their successful running, not sure worth mocking.

mathpluscode commented 3 years ago

Ok, in any case @YipengHu and I talked about closing this ticket soon because WandB should provide tests for their successful running, not sure worth mocking.

@NMontanaBrown you can skip these test requirements for now?

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.