juglab / n2v

This is the implementation of Noise2Void training.
Other
387 stars 107 forks source link

Gaby neubias #80

Closed turekg closed 4 years ago

turekg commented 4 years ago

In principle the yaml work is done. Still nebulous about the output with the "extra" quotes around the arrays. BUT I cannot find anywhere examples of values that are arrays. And I've tried all sorts of things to see if I can get rid of them. Anyways, we can go from here...

turekg commented 4 years ago

I've figured out why the test fails, it's because it reads the wrong input file... Will fix.

turekg commented 4 years ago

Apparently I am not allowed to upload a yaml file, so here is the content of one for testing with java (SEM 2D example)

language: python
framework: tensorflow
inputs:
  name: inputs
  axes: byxc
  data_type: float32
  data_range: '[-inf, inf]'
  shape:
    min: '[1, 4, 4, 1]'
    step: '[1, 4, 4, 0]'
outputs:
  name: activation_26
  axes: byxc
  data_type: float32
  data_range: '[-inf, inf]'
  halo: '[0, 22, 22, 0]'
  shape:
    scale: '[1, 1, 1, 1]'
    offset: '[0, 0, 0, 0]'
training:
  source: de.csbdresden.n2v.train.N2VTraining
  kwargs: '{"means": ["39137.754"], "stds": ["18713.775"], "n_dim": 2, "axes": "YXC",
    "n_channel_in": 1, "n_channel_out": 1, "unet_residual": false, "unet_n_depth":
    2, "unet_kern_size": 3, "unet_n_first": 32, "unet_last_activation": "linear",
    "unet_input_shape": [null, null, 1], "train_loss": "mse", "train_epochs": 2, "train_steps_per_epoch":
    2, "train_learning_rate": 0.0004, "train_batch_size": 128, "train_tensorboard":
    true, "train_checkpoint": "weights_best.h5", "train_reduce_lr": {"factor": 0.5,
    "patience": 10, "verbose": true}, "batch_norm": true, "n2v_perc_pix": 1.6, "n2v_patch_shape":
    [64, 64], "n2v_manipulator": "uniform_withCP", "n2v_neighborhood_radius": 5, "probabilistic":
    false}'
prediction:
  preprocess:
    spec: de.csbdresden.n2v.predict.N2VPrediction::preprocess
    kwargs: '{ mean: [39137.754], stdDev: [18713.775]}'
  postprocess:
    spec: de.csbdresden.n2v.predict.N2VPrediction::postprocess
    kwargs: '{ mean: [39137.754], stdDev: [18713.775]}'
tibuch commented 4 years ago

Looks good to me. Were you able to run the exported file in Fiji?

turekg commented 4 years ago

Looks good to me. Were you able to run the exported file in Fiji?

I am happy to test but I have no idea where to begin. Perhaps @frauzufall can do this quickly. I've added her as a reviewer

frauzufall commented 4 years ago

Sorry I hope to have everything ready to test in a couple of hours. But I have more requests without testing it :)

Are we sure about the language python there? Because this zip file cannot be used e.g. to do prediction in python, right? It was exported to match the Java side. I'm unsure. What do you think @tibuch?

turekg commented 4 years ago

I can make these changes very quickly. I will set the source for training to be the training class in python

Sorry I hope to have everything ready to test in a couple of hours. But I have more requests without testing it :)

* Can we name the created ZIP `export.modelzoo.zip`? This way I can associate a reader in Fiji without having to extract anything.

* Can we add `source` with the value `n2v` / `denoiseg` below `framework`? It is mentioned in the official spec, but not coherently. I want to use it to know which prediction to use automatically. It might move later somewhere else.

* you can remove `spec` from `preprocessing` and `postprocessing`, not sure if that is the correct entry there and we don't need it

* the `source` of `training` needs to be something different since it was trained in python. @tibuch maybe has an idea which identifier to put there.

Are we sure about the language python there? Because this zip file cannot be used e.g. to do prediction in python, right? It was exported to match the Java side. I'm unsure. What do you think @tibuch?

tibuch commented 4 years ago

I would take the N2V.train() method from python as training source.

The language should be python, because it is trained in python. At least that is how I understand it. And it could be used in python as well. We just don't have the functionality implemented.

I think for now this is okay.

turekg commented 4 years ago

Changes committed :0)

frauzufall commented 4 years ago

Ok, I'm testing it now, here is what's happing:

Then it works! Here is the version that is loaded without issues:

language: python
framework: tensorflow
inputs:
  - name: inputs
    axes: byxc
    data_type: float32
    data_range: [-inf, inf]
    shape:
      min: [1, 4, 4, 1]
      step: [1, 4, 4, 0]
outputs:
  - name: activation_26
    axes: byxc
    data_type: float32
    data_range: [-inf, inf]
    halo: [0, 22, 22, 0]
    shape:
      scale: [1, 1, 1, 1]
      offset: [0, 0, 0, 0]
training:
  source: de.csbdresden.n2v.train.N2VTraining
  kwargs: {"means": ["39137.754"], "stds": ["18713.775"], "n_dim": 2, "axes": "YXC",
    "n_channel_in": 1, "n_channel_out": 1, "unet_residual": false, "unet_n_depth":
    2, "unet_kern_size": 3, "unet_n_first": 32, "unet_last_activation": "linear",
    "unet_input_shape": [null, null, 1], "train_loss": "mse", "train_epochs": 2, "train_steps_per_epoch":
    2, "train_learning_rate": 0.0004, "train_batch_size": 128, "train_tensorboard":
    true, "train_checkpoint": "weights_best.h5", "train_reduce_lr": {"factor": 0.5,
    "patience": 10, "verbose": true}, "batch_norm": true, "n2v_perc_pix": 1.6, "n2v_patch_shape":
    [64, 64], "n2v_manipulator": "uniform_withCP", "n2v_neighborhood_radius": 5, "probabilistic":
    false}
prediction:
  preprocess:
    - spec: de.csbdresden.n2v.predict.N2VPrediction::preprocess
      kwargs: { mean: [39137.754], stdDev: [18713.775]}
  postprocess:
    - spec: de.csbdresden.n2v.predict.N2VPrediction::postprocess
      kwargs: { mean: [39137.754], stdDev: [18713.775]}

Extra questions:

Feels to me like we need another meeting for the details @turekg @tibuch

frauzufall commented 4 years ago

Thanks @turekg for already working on the other issues, we'll get there!! :sun_with_face:

frauzufall commented 4 years ago

Also, I asked the modelzoo people about the zip naming and they support using .bioimage.io.zip instead of their current .model.zip and our .modelzoo.zip, so that's what we should use too.

turekg commented 4 years ago
turekg commented 4 years ago

Okay, I have figured it out. In order to make array values appear within brackets (ie. [1,2,3,4]) without quotes I have to set the default output style to be one long string. Otherwise,the array above would show up as -1 -2 -3 -4 which is the correct representation of an array according to the spec. But that should be irrelevant to the reader....It only makes it less legible humans :-) I will implement this fix now

turekg commented 4 years ago

OK, I think this is a good as it's going to get. The zip file contains two yml files in two formats. Deboh confirms she's using a library so "in principle" it should be able to read either format. The only sticking point may be the long dump of config.json. Let me know. Unfortunately the plugin is not working for me it throws an error right away. Archive.zip

turekg commented 4 years ago

OK! It works now! I even added the new fields for the test images. @tibuch you will do the bit that creates the images?

tibuch commented 4 years ago

My bad! Removed an import by accident!

turekg commented 4 years ago

@tibuch after you did whatever you needed to do to my branch the builds fail with a memory error

terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
Fatal Python error: Aborted

in Travis, but they run OK on my Mac. I will try it on Jenkins.... Maybe also test with newer csbdeep?

tibuch commented 4 years ago

Sorry, I just rebased the branch onto master and accidentally removed an import. I have a running version on my system, but I am still working on the export.

Are you working on it as well? I could push my current version. Else I would push it once I am done with everything.

turekg commented 4 years ago

No go ahead push when you’re all done, but it’s not an import problem, I’ve fixed that. It’s something else. Maybe it’ll go away when you push again. gaby

On 23. Jun 2020, at 15:07, tibuch notifications@github.com wrote:

Sorry, I just rebased the branch onto master and accidentally removed an import. I have a running version on my system, but I am still working on the export.

Are you working on it as well? I could push my current version. Else I would push it once I am done with everything.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/juglab/n2v/pull/80#issuecomment-648133746, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKXJWKQXBOHUD22EKMBWHPDRYCSJFANCNFSM4N773L4Q.

tibuch commented 4 years ago

I changed version: 0.2.0-csbdeep to format_version: 0.2.0 to be compatible with the current java-produced yaml. I think eventually we will change it back to version: 0.2.0-csbdeep.

Additionally the test-images are now part of the output.

The structN2Vmask is exported as well, but Fiji complains if it is set to None.

tibuch commented 4 years ago

I forgot to update the tests after adding the changes to the exporter. Hopefully it works now.

The lzw-compression test fails on jug-srv1.

turekg commented 4 years ago

Make sure you have installed the extra library you need for tifffiles to read lzw compressed files. It is in setup.py

tibuch commented 4 years ago

hmm do I have to do this manually? I just install it with pip install -e . and then it doesn't seem to work.

frauzufall commented 4 years ago

@tibuch I updated the java code to set the version to 0.2.0-csbdeep

tibuch commented 4 years ago

How is the field called? version or format_version?

frauzufall commented 4 years ago

I think format_version. I'm on my phone, can't look

frauzufall commented 4 years ago

It's called format_version. The reference_input of the output node shape is missing:

  shape:
    reference_input: input
    scale: [1.0, 1.0, 1.0, 1.0]
    offset: [0, 0, 0, 0]
frauzufall commented 4 years ago

The name of the input tensor is input. The name of the output tensor is activation_11/Identity.

With these changes (adding reference_input and changes the tensor names), it works for me.

Also, min and shape don't look right... Can you verify these numbers?

tibuch commented 4 years ago

Make sure you have installed the extra library you need for tifffiles to read lzw compressed files. It is in setup.py

Used a python from outside of the env to run the test :facepalm:

tibuch commented 4 years ago

Thanks @frauzufall and @turekg.