search

colour-science / colour

Colour Science for Python
https://www.colour-science.org
BSD 3-Clause "New" or "Revised" License
2.11k stars 261 forks source link

Add tests for CIECAM02 reverse implementation. #104

Closed MichaelMauderer closed 10 years ago

MichaelMauderer commented 10 years ago

Okay I ran into the following problem: There are two example computation for CIECAM02 available from the website of Mark Fairchild

  1. http://rit-mcsl.org/fairchild/files/AppModEx.xls This contains forward computations for a large number of CAMs
  2. http://rit-mcsl.org/fairchild/files/CIECAM02function.XLS This contains forward and reverse computations for CIECAM02.

Currently we are using 1. for testing all the forward implementations of the CAMs and everything works fine. However, the reverse CIECAM does not agree with the data from the forward models. (Feeding in the output from those data-sets produces not the used input).

The current reverse CIECAM02 seems to follow the example from 2) and agrees with the values from the spreadsheet.

It seems there is some discrepancy between 1) and 2). They don't seem to produce the same results for the same input.

So the next step for me is to figure out where they differ (and double check, that I was not making some mistake inputing the data in the models, or overlooking some additional assumtpions the models are making)

KelSolaar commented 10 years ago

Huuu, that's fairly annoying, maybe worth contacting Fairchild about that when you have pinpointed what's going on.

KelSolaar commented 10 years ago

I remember when looking for implementation that I found one in Python, and there was a mention regarding an issue with Fairchild spreadsheet data, if it can be of any help:

https://github.com/njsmith/pycam02ucs/blob/eaac87f1b8eb076f45d4896a3be49cb3ecb3f2e8/pycam02ucs/ciecam02.py#L502

MichaelMauderer commented 10 years ago

Figured it out. Was an issue with an duplicate h/H value in the example dataset. The sheets are fine as is the implementation.

But I changed the signature of CIECAM02_to_XYZ. While it makes some sense to pass in the result of the XYZ_to_CIECAM02, the CIECAM02_Specification has many more attributes than the needed J, C and h.

KelSolaar commented 10 years ago

Yeah no problem!

I was passing the whole specification because there are ways to calculate missing arguments from others, and in an optimistic future I wanted to implement those computations. Let's address that when times come for it.

https://ritdml.rit.edu/bitstream/handle/1850/8424/YXueThesis10-2008.pdf?sequence=1 : 2.2.2 Inverse Model