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

Investigate colour differences of Smits (1999) recovered spectral power distributions. #196

Closed
KelSolaar opened this issue Jul 14, 2015 · 17 comments

Comments

@KelSolaar
Copy link
Member

This discussion is related to @henczati comment: #191 (comment)

Our implementation currently suffers from small discrepancies in regard to the original colour as seen in this IPython notebook: http://nbviewer.ipython.org/github/colour-science/colour-website/blob/master/ipython/about_reflectance_recovery.ipynb

I suspect it is related to the 1nm interpolation we are doing but it is of high interest for @henczati to find out the reason.

@KelSolaar
Copy link
Member Author

I will dive back into that shortly after our Machado (2010) CVD simulation implementation is in proper working order as I wanted to implement Meng, Simon and Hanika (2015) which is highly related to Smits (1999) as per #182.

I have restored a fully working Matlab version of Smits (1999) here: https://github.com/colour-science/smits1999 and would like to take that opportunity to thanks Brian for his contribution.

@henczati
Copy link

Much appreciated.
As the notebook is in a separate repo and currently I'm more interested in the notebook's code, I created the joint colour-science/colour-website#1 issue.

@henczati
Copy link

On this note, how well are the modules tested? Could there be a from __future__ import division missing or an unintended int/int occurring in something imported?

@KelSolaar
Copy link
Member Author

Could there be a from future import division missing or an unintended int/int occurring in something imported?

I don't think so as the code is very well tested (we are even testing for nan, infinity support for most of the functions where it makes sense) and I'm fairly sure that all the relevant modules (with definitions) have a from __future__ import division statement.

@henczati
Copy link

That's reassuring. :)
Keep up the good work. ;)

@KelSolaar
Copy link
Member Author

Allright!

The problem is happening even before any chromatic adaptation steps as the tristimulus values of original and recovered spd are already not matching perfectly:

===============================================================================
*                                                                             *
*   "dark skin" - Reflectance Recovery                                        *
*                                                                             *
===============================================================================
(array([ 0.1193411 ,  0.09994307,  0.05593965]), array([ 0.11536937,  0.09682546,  0.05603308]))
===============================================================================
*                                                                             *
*   "light skin" - Reflectance Recovery                                       *
*                                                                             *
===============================================================================
(array([ 0.41222281,  0.36318084,  0.23640395]), array([ 0.40200177,  0.35527799,  0.23727932]))
===============================================================================
*                                                                             *
*   "blue sky" - Reflectance Recovery                                         *
*                                                                             *
===============================================================================
(array([ 0.18268199,  0.18754091,  0.31766449]), array([ 0.18477136,  0.18804614,  0.31247735]))
===============================================================================
*                                                                             *
*   "foliage" - Reflectance Recovery                                          *
*                                                                             *
===============================================================================
(array([ 0.10650312,  0.12892686,  0.06107233]), array([ 0.10917658,  0.13234666,  0.06489246]))
[...]

It is actually probably related to the interpolation we are doing, let me explain:

Smits (1999) base spds are provided with the following shape:

print(colour.SMITS_1999_SPDS['White'].shape)
(380.0, 720.0, 37.7777)

Notice the bins / steps size: 37.7777 (which actually tend to infinity)

Our CIE 1931 2 Degree Standard Observer CMFS have the following shape:

print(colour.CMFS['CIE 1931 2 Degree Standard Observer'].shape)
(360, 830, 1)

In order to perform the products before the integrals (summation) in colour.spectral_to_XYZ (https://github.com/colour-science/colour/blob/develop/colour/colorimetry/tristimulus.py#L99), we ensure that the shapes of the spectral elements involved are compatible. The way it is done usually is by either interpolating or better by zero filling the missing bins (faster and it doesn't change the output at all, which can be the case with interpolation depending the interpolator you choose).

Now it is unfortunate but Smits (1999) base spds cannot be zeros filled properly against the CIE 1931 2 Degree Standard Observer CMFS because of the steps size. As a result I'm aligning the resulting spd to CIE 1931 2 Degree Standard Observer shape which slightly alter its quality as you cannot get the exact same curve.

I took that opportunity to raise an exception in colour.SpectralPowerDistribution.zeros method as it was silently failing if unsuccessful: 66ec062

Now that being said I found some other issues related to fractional numbers and the way we retrieve the related wavelength values.

@henczati
Copy link

I might have found a typo: Why is the 3rd element of the 2nd tuple spd_shape.end and not spd_shape.steps in the boundaries definition in SpectralPowerDistribution.zeros?

@henczati
Copy link

Thanks for fixing SPD.zeros, btw.

Looking at some function implementations, I also have the impression, that CMFS usage might not be consistent. Some functions allow it to be specified, some use a hard-wired CMFS (e.g. I've seen the 1931 2° observer used hard-wired, and not by a default-argument definition). Is it possible and if yes, is there any safeguard against or warning for using them in a non-matching way? Could this also contribute to the problem?

@KelSolaar
Copy link
Member Author

I might have found a typo: Why is the 3rd element of the 2nd tuple spd_shape.end and not spd_shape.steps in the boundaries definition in SpectralPowerDistribution.zeros?

Good catch, it is a typo, although without apparent consequences as the main purpose of colour.SpectralPowerDistribution.zeros is to math the steps size of another spd which means that everytime it is used a colour.SpectralShape class with steps="some number" is passed, the boundaries= statement was just a safe guard in the case somebody wanted to pass a colour.SpectralShape without any fields set.

@KelSolaar
Copy link
Member Author

I also have the impression, that CMFS usage might not be consistent. Some functions allow it to be specified, some use a hard-wired CMFS (e.g. I've seen the 1931 2° observer used hard-wired, and not by a default-argument definition).

Do you have an example where it is the case? Usually if you can't pass an arbitrary CMFS, it is because the algorithm / method was based on a very specific one.

Is there any safeguard against or warning for using them in a non-matching way?

There is nothing like that, it's either the CMFS usage is bound to one specific or open to any.

@henczati
Copy link

Do you have an example where it is the case?

IIRC, it was some plotting/displaying function. I saw it somewhere 1 or 2 days ago, but when writing about it, I couldn't find it. If I stumble upon it again, I'll make a note.

@KelSolaar
Copy link
Member Author

Allright! Let me know, I have a related old issue: #25

It is quite a complex subject, for example can you use a colourspace the same way with the 2 degrees and 10 degrees observers? Would you change the primaries accordingly as the white point is changed? How would you compute the new primaries as most of the RGB colourspaces don't have associated primaries spds? Etc...

@KelSolaar
Copy link
Member Author

It is kind of unfortunate but Smits (1999) Matlab Code doesn't produce the same basis functions than the paper, they are slightly different and as a result exacerbate the colour discrepancies.

@henczati
Copy link

Allright! Let me know

Found one in colour_quality_scale

Smits (1999) Matlab Code doesn't produce the same basis functions than the paper

Huhh, and one would think that scientists strive to be precise...can't believe what anyone says these days... :D

@KelSolaar
Copy link
Member Author

Found one in colour_quality_scale

Ah yes! Ohno (2010) specifically uses the CIE 1931 2 Degree Standard Observer CMFS, I'm not sure it is something you can change without having to tweak some other factors, hence the fact it is not exposed as a parameter. I could ask Yoshi about it actually.

Huhh, and one would think that scientists strive to be precise...can't believe anyone these days... :D

It is maybe some discrepancies in Matlab itself too, a case of bitrot.

@henczati
Copy link

bitrot

I did not think of that. :) Every day a new lesson. :)

@KelSolaar
Copy link
Member Author

It is maybe not that but without Brian help it will prove to be tough to know where it's coming from, I had to find online various deprecated Matlab functions to have his code working again. The two first generated curves (White and Cyan) match but the remaining ones don't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants