core-bioimage-io-python
core-bioimage-io-python copied to clipboard
Better default io names in build_model()
... Does it check for the names of the input&output layers when writing the rdf.yaml?
Originally posted by @esgomezm in https://github.com/bioimage-io/collection-bioimage-io/issues/571#issuecomment-1485578142
We could inspect the (TF) weights for better tensor_inputs
and tensor_outputs
default names in build_model()
input/output layer names are important (https://github.com/bioimage-io/collection-bioimage-io/issues/571#issuecomment-1486480847) , but not always very informative or dependent on the architecture "conv2d_19".
I wonder if we shouldn't add them as fields to the corresponding weights entries (tensorflow_saved_model_bundle
, keras_hdf5
?, tensorflow_js
?).
This would be easy to test and could be generated by bioimageio.core for older format versions lacking this information. updating models with this information would be easy.
@carlosuc3m @esgomezm @constantinpape should we separate this as a technical detail from the existing input/output names?
It could be but I'm not sure how to avoid a misunderstanding between the current input
and output
fields
I think I would prefer to have it in separate fields for the respective weight formats. But I don't have a strong opinion here, as I am not actively using any of these weight formats, so feel free to implement whichever solution works best for you.
To illustrate the separation of the fields:
inputs: [{name: raw_input, ...}]
outputs: [{name: prediction, ...}]
weights:
tensorflow_saved_model_bundle:
input_layer_names: [input_1]
output_layer_names: [conv2d_19]
edit: changed input_layer_name to input_layer_names list
developers using specific weights would find the layer names right there. higher-level names for model users (using our libraries) can abstract away the layer names, which are weight format dependent...
why dont' we like to name the inputs and outputs with names like conv2d_19
or input_1
?
I always thought of the input and output tensor names as content descriptions, raw_input
, em_volume
, or even something like left_camera
and right_camera
for a two camera setup... i.e. identifiers you would use in code for those tensors.
while input_1
is sufficiently generic to be anything, conf2d_19
is clearly the name of a convolutional layer --- no good name for the output tensor in my opinion. Add any of the optional postprocessing steps and the name is quite misleading. (note that we have general issues with the tensor referencing pre- and post any pre- or postprocessing step.)
A bit of a vague point, but it feels like mixing implementation detail with the abstraction layer defined by the RDF.
And along those lines: In the model RDF all weight format specific details are in their respective weights
entry. What if ONNX layers will also need names?
one more scenario to illustrate the "mix" (yes, it's a bit of a contructed issue 😇 ): what if a model has pytorch an onnx weights with arbitrary names. In a new model version converted tf weights are added. This would entail that inputs and outputs need to be renamed, which is weird.
That is a good point, but then I whink that we need a way to know which of the inputs of the model correspond to each of the inputs specified in the yaml
That is a good point, but then I whink that we need a way to know which of the inputs of the model correspond to each of the inputs specified in the yaml
can't we simply list the network input names in the order of the model inputs? could you give a counter-example? or how could we define this instead? if we define it per weight format we can adjust for each format if necessary...
can't we simply list the network input names in the order of the model inputs? could you give a counter-example? or how could we define this instead? if we define it per weight format we can adjust for each format if necessary...
As long as the order is kept the same I think that your suggestion will be enough yes @FynnBe . Is this already implemented in the specs?
As long as the order is kept the same I think that your suggestion will be enough yes @FynnBe . Is this already implemented in the specs?
Hi @carlosuc3m , no it's not implemented yet, but I can make a draft today (edit: given that I find where those requirements are documented). You mention in https://github.com/bioimage-io/collection-bioimage-io/issues/540#issuecomment-1557647179 that ONNX actually requires specific input names, as well. Is this documented somewhere? Can one not simply provide the input tensors as positional arguments?
Same for TensorFlow: I'd like to make sure that we don't have the option of providing input tensors positionally. Where does this break exactly? (@esgomezm you encountered this (https://github.com/bioimage-io/collection-bioimage-io/issues/571#issuecomment-1486480847)... is this documented somewhere? ) For TensorFlow Javascript I only found
...
const image = util.textToImageArray(imageText, this.imageSize);
const predictOut = this.model.predict(image);
from https://www.tensorflow.org/js/tutorials/conversion/pretrained_model#explore_the_code which seems to get away without any tensor names. Is this specific to TensorFlow 1?
@carlosuc3m in https://github.com/bioimage-io/collection-bioimage-io/issues/609#issuecomment-1610156170 how do you know the onnx names that you would need? Can we solve this with inspecting the onnx weights or finding another way to provide positional arguments? Adding the weights specific tensor name fields as mandatory fields feels a bit of an overkill, espcially because running, e.g. the onnx model in Python does not require those tensor names.