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

Simplify CAREamist BMZ export #144

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Simplify CAREamist BMZ export #144

merged 1 commit into from
Jun 13, 2024

Conversation

jdeschamps
Copy link
Member

Description

Following a chat with @CatEek, I've come to realize that my previous attempt to pull patches from the dataloaders in order to export the model to the BMZ format was difficult to read (patches needed to be denormalized in order to be put through the BMZ pipeline).

I've decided to just force users to input an array. This can be the training or prediction arrays, this is a simple solution to having a clearer code to maintain,

  • What: Removed CAREamist._create_data_for_bmz, input_array now mandatory for CAREamist.export_to_bmz.
  • Why: Pulling patches from the dataloaders in order to avoid the input of an array led to complex code.
  • How: See what.

Changes Made

  • Modified: CAREamist.export_to_bmz.
  • Removed: CAREamist._create_data_for_bmz and corresponding tests.

Breaking changes

Any code not inputing input_array, e.g. all notebooks examples.

Currently, if the array does not have the same dimensions/axes as what the configuration states, users should get an error from the reshape function.

Additional Notes and Examples

Before, after training a model or loading a model, the following code would run:

careamist.export_to_bmz(
    path="sem_n2v_model.zip",
    name="SEM_N2V",
    authors=[{"name": "CAREamics authors", "affiliation": "Human Technopole"}],
)

It would create input data for the BMZ, using the following steps:

  • If there is a prediction dataloader, pull a patch from it and denormalize it
  • If there is a training dataloader, pull a patch from it and denormalize it
  • If there is not dataloader, create a random array using the mean and std in the configuration

Now, users have to provide an input array:

careamist.export_to_bmz(
    path="sem_n2v_model.zip",
    name="SEM_N2V",
    input_array=some_array, 
    authors=[{"name": "CAREamics authors", "affiliation": "Human Technopole"}],
)

Please ensure your PR meets the following requirements:

  • Code builds and passes tests locally, including doctests
  • New tests have been added (for bug fixes/features)
  • Pre-commit passes
  • PR to the documentation exists (for bug fixes / features)

@jdeschamps jdeschamps requested review from CatEek and melisande-c June 13, 2024 16:20
@jdeschamps jdeschamps merged commit aa66daf into main Jun 13, 2024
15 checks passed
@jdeschamps jdeschamps deleted the jd/fix/simplify_bmz branch June 13, 2024 16:51
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.

1 participant