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

Add cividis colormap #1546

Merged
merged 2 commits into from
Jan 24, 2018
Merged

Add cividis colormap #1546

merged 2 commits into from
Jan 24, 2018

Conversation

NichtJens
Copy link
Contributor

This adds cividis as 113th colormap to TColor, as well as the palettes tutorial.

This colormap aims to solve problems that people with color vision deficiency have with the common colormaps. For more details see:

Nuñez J, Anderton C, and Renslow R. Optimizing colormaps with consideration for color vision deficiency to enable accurate interpretation of scientific data.

https://arxiv.org/abs/1712.01662

The colormap stops have been interpolated from the (256 stops) Fiji/ImageJ version [1] and cut down to the 9 stops that ROOT uses (using the scipy Akima interpolator). Alternatively, the official* (18 stop) version committed to plotly.py [2] could be used, which would mean a slight deviation from the other palettes (see code below). In the plot produced by the palette tutorial the difference between the 9 and 18 stops seems negligible, hence sticking to the ROOT default values seems appropriate.

Double_t cstps[18] = {0.0, 0.0588235294118, 0.117647058824, 0.176470588235, 0.235294117647, 0.294117647059, 0.352941176471, 0.411764705882, 0.470588235294, 0.529411764706, 0.588235294118, 0.647058823529, 0.705882352941, 0.764705882353, 0.823529411765, 0.882352941176, 0.941176470588, 1.0};
Double_t red[18]   = {  0./255.,   0./255.,   0./255., 39./255.,  60./255.,  76./255.,  91./255., 104./255., 117./255., 131./255., 146./255., 161./255., 176./255., 192./255., 209./255., 225./255., 243./255., 255./255.};
Double_t green[18] = { 32./255.,  42./255.,  52./255., 63./255.,  74./255.,  85./255.,  95./255., 106./255., 117./255., 129./255., 140./255., 152./255., 165./255., 177./255., 191./255., 204./255., 219./255., 233./255.};
Double_t blue[18]  = { 76./255., 102./255., 110./255., 108./255., 107./255., 107./255., 109./255., 112./255., 117./255., 120./255., 120./255., 118./255., 114./255., 109./255., 102./255., 92./255.,  79./255.,  69./255.};
Idx = TColor::CreateGradientColorTable(18, cstps, red, green, blue, 255, alpha); 

*Meaning committed by the original author.

[1] https://github.com/fiji/fiji/blob/master/luts/cividis.txt
[2] plotly/plotly.py#883

@phsft-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@pcanal
Copy link
Member

pcanal commented Jan 22, 2018

@phsft-bot build

@phsft-bot
Copy link
Collaborator

Starting build on centos7/gcc49, mac1013/native, slc6/gcc49, slc6/gcc62, slc6/gcc62, ubuntu16/native, ubuntu16/native, windows10/vc15 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on mac1013/native.
See console output.

Warnings:

  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/R.h:40:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/R_ext/RS.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/Rinternals.h:60:11: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/library/Rcpp/include/Rcpp/r/headers.h:57:10: warning: non-portable path to file '<RVersion.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]
  • /cvmfs/sft.cern.ch/lcg/releases/R/3.2.5-c8cca/x86_64-mac1013-clang90-opt/lib/R/include/R_ext/Visibility.h:29:10: warning: non-portable path to file '<RConfig.h>'; specified path differs in case from file name on disk [-Wnonportable-include-path]

And 10 more

Failing tests:

@NichtJens
Copy link
Contributor Author

From the output, it doesn't seem the build failure was caused by my changes. I do not have access to the full log, though. So, if I need to fix something it would be nice if someone with access could provide the full output...

@pcanal
Copy link
Member

pcanal commented Jan 23, 2018

Yes, the error is unrelated to your patch (infrastructure problem).

@pcanal
Copy link
Member

pcanal commented Jan 23, 2018

@couet Could you take a look at this PR? Thanks.

@couet couet self-assigned this Jan 23, 2018
@couet couet merged commit b7013ac into root-project:master Jan 24, 2018
@couet
Copy link
Member

couet commented Jan 24, 2018

After fixing two small mistakes in your code the new palette is now working. Thanks !

@NichtJens
Copy link
Contributor Author

Great!

I missed the array dimension from when I compared to the 18 stop version... Sorry for that.

The 62 -> 63 change escaped me completely. It might be better to calculate total number minus first number (113 - 50) with named constants instead of magic numbers to make this more visible...

@NichtJens NichtJens deleted the add_cividis branch January 24, 2018 14:32
@couet
Copy link
Member

couet commented Jan 24, 2018

Yes 63 can be calculated but in that case 113 will need to be set. Yes we can have a named constant but the comment on the ligne setting 63 is clear enough I guess.

@couet
Copy link
Member

couet commented Jan 24, 2018

Yes adding a last dummy palette number might help to compute the value automatically. But then it will appear in the enum description in the online help ... That's not very clean ...
If you don't mind I guess we can leave it like that right now.

@NichtJens
Copy link
Contributor Author

Certainly. I was indeed thinking in the direction of your second comment... But, sure, it's not the highest priority.

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.

4 participants