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

Image Normalization #160

Merged
merged 9 commits into from
May 29, 2021
Merged

Image Normalization #160

merged 9 commits into from
May 29, 2021

Conversation

Michael-F-Bryan
Copy link
Contributor

Closes #159.

Copy link
Contributor

@Mi1ind Mi1ind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Michael-F-Bryan Tried building mobilenet_non_quantized with the proc_block. I am getting the following error when running rune build Runefile:

error[E0599]: no method named `set_count` found for struct `MostConfidentIndices` in the current scope
  --> lib.rs:23:20
   |
23 |     most_confident.set_count(3i32);
   |                    ^^^^^^^^^ help: there is an associated function with a similar name: `with_count`

error[E0599]: no method named `set_labels` found for struct `Label` in the current scope
  --> lib.rs:25:13
   |
25 |     label . set_labels (["background" , "tench/Tinca_tinca" , "goldfish/Carassius_auratus" , "great_white_shark/white_shark/man_eater/man...
   |             ^^^^^^^^^^ help: there is an associated function with a similar name: `with_labels`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0599`.
error: could not compile `mobilenet_non_quantized`

To learn more, run the command again with --verbose.
Error: Rune compilation failed

Caused by:
    0: Compilation failed
    1: Compilation failed
    2: Cargo exited with a return code of 101

Rune version:

rune 0.2.1 (574b452 2021-05-23)

@Michael-F-Bryan
Copy link
Contributor Author

Michael-F-Bryan commented May 26, 2021

@Mi1ind, I did some digging and it looks like you may have been using an older version of the hotg-ai/rune repo as a git dependency. In b7faf35, the MostConfidentIndices type was switched from using a with_count() builder method to a set_count() setter.

The change itself was made (and merged) on 2021-05-21.

The quick'n'dirty way to make this go away would be to delete the rune repo checkouts (rm -r ~/.cargo/git/checkouts/rune-*). Otherwise you could delete the ~/.cache/runes/ folder (or wherever runes are built on your machine) and the next time you build a rune it'll fetch the latest commits from GitHub.

If neither of those options work, can you make a ticket? I don't the issue has anything to do with image normalization.

@Mi1ind
Copy link
Contributor

Mi1ind commented May 26, 2021

@Michael-F-Bryan I was able to successfully build the Rune by rebasing the current branch locally with master.

The output of the quantized vs non-quantized mobilenet runes are quite different however. The non quantized version does not display the expected outputs:

Quantized Mobilenet (Expected Output)
quantized

Non Quantized Mobilenet (Uses normalized data)
non-quantized

QR Code to run and deploy non-quantized mobilenet:
qr_code

Quick Links
Non-Quantized Mobilenet Runefile
TensorFlow Hub Link to Non-Quantized Model

@Michael-F-Bryan
Copy link
Contributor Author

The output of the quantized vs non-quantized mobilenet runes are quite different however. The non quantized version does not display the expected outputs.

I'd suggest checking your Runefile. You'll probably find that the non-quantized version isn't sending the scores to an output.

@Mi1ind
Copy link
Contributor

Mi1ind commented May 27, 2021

The output of the quantized vs non-quantized mobilenet runes are quite different however. The non quantized version does not display the expected outputs.

I'd suggest checking your Runefile. You'll probably find that the non-quantized version isn't sending the scores to an output.

@Michael-F-Bryan, I apologize, should have been more clear with the error -

Output from mobilenet using the quantized model (expected output) when taking a picture of my fridge is {turnstile, refrigerator/icebox, microwave/microwave_oven}. This is pretty accurate.

Output from mobilenet using the non-quantized model (uses image-normalization proc block) with the same picture is {window shade, theatre curtain, shower curtain}. This does not match the expected output.

The minimum and maximum input for the model based on its metadata seems to be [-1.0, 1.0]. Is the image-normalization processing block only outputting positive integers to the model?

@Michael-F-Bryan
Copy link
Contributor Author

Is the image-normalization processing block only outputting positive integers to the model?

This proc block will normalize pixel values based on a standard normal distribution, not (value-min)/(max-min). That means it could give you arbitrarily large values (well technically ±256/2/σ for a u8-based image).

That said, if you've told the SERIAL output it has two inputs then both inputs should be sent back to the runtime. It may be that the app isn't displaying them correctly, or one input is longer than the other so they can't be matched up into pairs.

Does the mobile app have a debug log you can inspect to see the exact data sent to the serial output?

@Mi1ind
Copy link
Contributor

Mi1ind commented May 28, 2021

Looked into this a bit more, normalized the input image using mean: 127.5 and standard deviation: 127.5. The proc block converts pixel values to the expected z-score (i.e. 255.0 -> 1.0). Couple of questions -

  1. Is the noop test in lib.rs supposed to fail? This is the result of cargo test:
test tests::normalizing_with_default_distribution_is_noop ... FAILED

failures:

---- tests::normalizing_with_default_distribution_is_noop stdout ----
thread 'tests::normalizing_with_default_distribution_is_noop' panicked at 'index out of bounds: the index was [2, 2, 0] but the tensor has dimensions of [3, 4, 1]', proc_blocks/image-normalization/src/lib.rs:113:33
  1. How do I set the distribution values in the Runefile.yml (mean, standard deviation)? Currently I'm changing the default values in the proc block.

That said, if you've told the SERIAL output it has two inputs then both inputs should be sent back to the runtime. It may be that the app isn't displaying them correctly, or one input is longer than the other so they can't be matched up into pairs.

Does the mobile app have a debug log you can inspect to see the exact data sent to the serial output?

The mobile app does not have a debug log, but I was able to work around this by sending the model output directly to serial and checked if the correct indices are being paired with the labels. The app seems to be matching them correctly.

@Michael-F-Bryan
Copy link
Contributor Author

Is the noop test in lib.rs supposed to fail?

Nope.

I ran into an issue where index calculations (e.g. going from [1, 2, 3] to the correct element in the backing array) were being done incorrectly for tensors with more than 2 dimensions. This was in the lead-up to Kartik's talk, so I put this PR on hold to write up docs.

How do I set the distribution values in the Runefile.yml (mean, standard deviation)? Currently I'm changing the default values in the proc block.

The Distribution type models a single normal distribution, but you want to set properties on the proc block itself.

You can do this for individual channels or for all three at the same time. A Distribution can be created from a 2-element array of floats, so you'd use [mean, std_dev] as the arguments.

For example:

pipeline:
  image_normalize:
    proc_block: hotg-ai/rune#proc_blocks/image-normalization
    args:
      red: [127.5, 64.0]
      rgb: [1.0, 0.0]

I'll need to update the code to use set_rgb() instead of with_rgb(), though.

@Michael-F-Bryan
Copy link
Contributor Author

@Mi1ind, I believe I've fixed the indexing error. Can you check out the latest version of this branch and see whether it works with mobilenet?

@Michael-F-Bryan Michael-F-Bryan marked this pull request as ready for review May 29, 2021 08:06
@Mi1ind
Copy link
Contributor

Mi1ind commented May 29, 2021

@Michael-F-Bryan
I updated with_rgb method to set_rgb locally, and am running into the following error when building the Runefile.yml:

[2021-05-29T16:04:51.379Z DEBUG rune_codegen::environment] Executing "cargo" "+nightly-2021-05-09" "build" "--target=wasm32-unknown-unknown" "--quiet" "--release"
error[E0382]: borrow of moved value: `normalize`
  --> lib.rs:29:5
   |
20 |     let mut normalize = image_normalization::ImageNormalization::default();
   |         ------------- move occurs because `normalize` has type `ImageNormalization`, which does not implement the `Copy` trait
21 |     normalize.set_rgb([127.5f32, 127.5f32]);
   |               ----------------------------- `normalize` moved due to this method call
...
29 |     normalize.set_output_dimensions(&[3usize, 224usize, 224usize]);
   |     ^^^^^^^^^ value borrowed here after move
   |
note: this function takes ownership of the receiver `self`, which moves `normalize`
  --> /Users/milind/GitHub/hotg-ai/rune/proc_blocks/image-normalization/src/lib.rs:21:23
   |
21 |     pub fn set_rgb<D>(self, distribution: D) -> Self
   |                       ^^^^

error: aborting due to previous error

The same error persists even after adding Copy trait to lib.rs:

    pub fn set_rgb<D>(self, distribution: D) -> Self
    where
        D: TryInto<Distribution>,
        D::Error: Display,
        D:Copy,
    {

This is my proc bloc:

  normalize:
    proc-block: "hotg-ai/rune#proc_blocks/image-normalization"
    inputs:
      - image
    outputs:
      - type: F32
        dimensions:
          - 3
          - 224
          - 224
    args:
      rgb: [127.5, 127.5]

Built the rune by manually modifying the default values, the model output doesn't seem to be too accurate. The image below is a screenshot of the non-quantized mobilenet. notebook/notebook_computer is the 8th most likely result, when it should be the 1st.
IMG_0001

@Michael-F-Bryan
Copy link
Contributor Author

Sorry, I should have explained that a bit better. When we went from with_rgb() to set_rgb() it asn't just a name change, we switched from using a builder method (take self by value and return a new instance) to a setter (take &mut self and mutate the field in-place, possibly returning &mut Self so the setters can be changed).

@Michael-F-Bryan Michael-F-Bryan merged commit f7bc6c0 into master May 29, 2021
@Michael-F-Bryan Michael-F-Bryan deleted the 159-rgb-normalize branch May 29, 2021 17:32
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.

Proc block for normalizing RGB images
2 participants