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

roundtrip hed -> rgb -> hed is broken #6104

Closed
cgebbe opened this issue Dec 6, 2021 · 8 comments
Closed

roundtrip hed -> rgb -> hed is broken #6104

cgebbe opened this issue Dec 6, 2021 · 8 comments
Labels
📄 type: Documentation Updates, fixes and additions to documentation

Comments

@cgebbe
Copy link

cgebbe commented Dec 6, 2021

Description

It seems the roundtrip hed -> rgb -> hed has been broken recently:

Way to reproduce

  img_in = np.random.uniform(size=(10,10,3))
  img_out = skimage.color.rgb2hed(skimage.color.hed2rgb(img_in))
  np.testing.assert_array_almost_equal(img_in, img_out)  #  throws an error

Version information

3.8.10 (tags/v3.8.10:3d8993a, May  3 2021, 11:48:03) [MSC v.1928 64 bit (AMD64)]
Windows-10-10.0.19042-SP0
scikit-image version: 0.19.0
numpy version: 1.20.3

Possible root cause

I believe that the root cause is the added np.maximum, see https://github.com/scikit-image/scikit-image/blame/main/skimage/color/colorconv.py#L1578

Existing test

There already is a test for hed->rgb->hed conversion, namely https://github.com/scikit-image/scikit-image/blob/v0.19.x/skimage/color/tests/test_colorconv.py#L241 . However, self.img_stains only has float values until 0.3. We can make the test above pass by enforcing...

    img_in = np.random.uniform(size=(10,10,3)) * 0.3   # note the additional 0.3!
    img_out = skimage.color.rgb2hed(skimage.color.hed2rgb(img_in))
    np.testing.assert_array_almost_equal(img_in, img_out)  # passes
@mkcor
Copy link
Member

mkcor commented Dec 7, 2021

Hello @cgebbe,

I believe that the root cause is the added np.maximum, see https://github.com/scikit-image/scikit-image/blame/main/skimage/color/colorconv.py#L1578

🤔 I wouldn't think so, since img_in contains no negative value... So we wouldn't expect img_out to contain negative values either...

@rfezzani
Copy link
Member

rfezzani commented Dec 8, 2021

Thank you @cgebbe for reporting, I can reproduce!

@rfezzani
Copy link
Member

rfezzani commented Dec 8, 2021

OK, it seems that the round trip breaking was expected: please see #5164 (comment) and it seems that you are right @cgebbe, np.maximum is causing this.

@rfezzani
Copy link
Member

@scikit-image/core, do we consider this is a bug or an expected behavior? Should we close?

@mkcor
Copy link
Member

mkcor commented Dec 13, 2021

At this point, I would consider it expected behaviour, but we should say something about it in the docstring. So maybe relabel this issue from 'bug' to 'documentation?'

My understanding is that there is no perfect solution, from what @crisluengo explained ("Every lab produces different colors for the same stains..."), also in this more recent thread: https://github.com/scikit-image/scikit-image/discussions/6065#discussioncomment-1708113

@alexdesiqueira
Copy link
Member

@rfezzani @mkcor I think that we should deal with this as a documentation "issue" right now. However, I will get back to this at some point in time and check how other packages deal with this (not in the near future, for sure 🙂).

@rfezzani rfezzani added 📄 type: Documentation Updates, fixes and additions to documentation and removed type: bug labels Jan 20, 2022
@crisluengo
Copy link
Contributor

@cgebbe There are colors that cannot be represented in this HED "color space". HED represents the combination of three stains commonly used in microscopy: Hematoxylin (blue/purple), Eosin (pink) and DAB (brown). There is no way to combine these three stains to obtain, for example, green. So in your input random image, any greenish pixel, when converted to HED, will necessarily change color.

Conversion from RGB to HED should be applied only to images of tissue stained with these stains. Conversion from HED to RGB can be used to simulate what such a stained tissue would look like. Don't use HED as a generic color space such as RGB, CMYK, Yxy, Lab, etc.

@grlee77
Copy link
Contributor

grlee77 commented Feb 25, 2022

I am going to close this as answered. It does not seem that new changes are necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 type: Documentation Updates, fixes and additions to documentation
Projects
None yet
Development

No branches or pull requests

6 participants