core-bioimage-io-python icon indicating copy to clipboard operation
core-bioimage-io-python copied to clipboard

Plans for weight converter? Common interface.

Open constantinpape opened this issue 4 months ago • 2 comments

There have been some recent changes to the weight converters and it seems not everything there is up-to-date.

In weight_converter/torch/_torchscript the function converts the weights and returns the corresponding weight description: https://github.com/bioimage-io/core-bioimage-io-python/blob/main/bioimageio/core/weight_converter/torch/_torchscript.py#L98

In weight_converter/torch/_onnx the function name implies that the weights would directly be added to the model, but that does not seem to happen in the function. Note that this initially followed the same design as in the torchscript converter when I implemented this. See https://github.com/bioimage-io/core-bioimage-io-python/blob/main/bioimageio/core/weight_converter/torch/_onnx.py

In weight_converter/keras/_tensorflow the function is similar to the torchscript one above, but it does not return the weight description. See https://github.com/bioimage-io/core-bioimage-io-python/blob/main/bioimageio/core/weight_converter/keras/_tensorflow.py#L101

It would be good to agree to a common syntax for this functionality and outline a plan to update this and make it usable again. I would prefer to follow the syntax in the torchscript file; adding the weight directly to the model could be added as a different function that makes use of the conversion functions internally.

constantinpape avatar Apr 29 '24 19:04 constantinpape

Totally! These converter submodules should only need minor updates (with the option to add additional converters). Providing both functionalities as you suggest makes sense to me 👍 Anyone interested in contributing is welcome to tackle this. I would do my best to provide answers to any questions that may arise.

FynnBe avatar Apr 30 '24 07:04 FynnBe

@bioimage-io/spec-dev anyone interested in taking a look?

FynnBe avatar Apr 30 '24 07:04 FynnBe