Gradient SVG is slightly inaccurate

[fedarko]

The actual coloring of samples is fine, I think, but the SVG gradients produced sometimes cover only ~99% of the domain, not 100%. This means that the end colors shown in the gradients are very slightly incorrect.

It's pretty confusing, because right now there are two tests that test the gradient SVG creation:

1. In one of these tests this error is present. The returned Viridis gradient, which is used as the reference for verifying the result, ends at 99% (with #f8e725), rather than at 100% (#fee825, which is the actual end color for Viridis).

2. In the other one of these tests, this error does not seem to be present: the color map being tested is Blues, and the minimum and maximum colors (#f7fbff and #08306b) match up with the extrema for this color map (going off what chroma.scale('blues')(0) and chroma.scale('blues')(1) return).

I think the problematic part of code is within this block:

emperor/emperor/support_files/js/color-view-controller.js

Lines 426 to 439 in b625a54

	//build the SVG showing the gradient of colors for values
	var mid = (min + max) / 2;
	var step = (max - min) / 100;
	var stopColors = [];
	for (var s = min; s <= max; s += step) {
	stopColors.push(interpolator(s).hex());
	}
	var gradientSVG = '<defs>';
	gradientSVG += '<linearGradient id="Gradient" x1="0" x2="0" y1="1" y2="0">';
	for (var pos = 0; pos < stopColors.length; pos++) {
	gradientSVG += '<stop offset="' + pos + '%" stop-color="' +
	stopColors[pos] + '"/>';
	}
	gradientSVG += '</linearGradient></defs><rect id="gradientRect" ' +

The first for loop seems suspicious to me. I'm not completely sure yet what's wrong with this code, but it looks like there might be precision issues with the s <= max check: since the data used for the test that failed had a step of (4 - 0) / 100 = 0.04, while the data used for the test that succeeded had a relatively larger step of (50 - 0) / 100 = 0.5.

I think it would make the most sense to just remove some of this code and use chroma.scale.colors() to do the work for us:

 var stopColors = interpolator.colors(101);
// Now we can construct gradientSVG just like we did before, with the final color (corresponding
// to the maximum value) being placed at 100%

Can put together a PR for this tomorrow if you'd like.

This impacts Empress as well, since the gradient SVG code there was adapted from this code.

[ElDeveloper]

Thanks for tracking this down. Yes, it would be very helpful if you can put together a PR.

…

On (Oct-13-20|20:03), Marcus Fedarko wrote: The actual coloring of samples is fine, I think, but the SVG gradients produced _sometimes_ cover only ~99% of the domain, not 100%. This means that the end colors shown in the gradients are very slightly incorrect. It's pretty confusing, because right now there are two tests that test the gradient SVG creation: 1. In [one of these tests](https://urldefense.com/v3/__https://github.com/biocore/emperor/blob/b625a5403ce9ad0ef3a84c6bc0b68b33e97888a4/tests/javascript_tests/test_color_view_controller.js*L212-L325__;Iw!!Mih3wA!VeKZkpvzEILHoclhWPbUFi5mPuFp3ezfo9QqGvYAozXbwgUZKc2TQB0463FvbfM$ ) this error is present. The returned Viridis gradient, which is used as the reference for verifying the result, ends at 99% (with `#f8e725`), rather than at 100% (`#fee825`, which is the actual end color for Viridis). 2. In [the other one of these tests](https://urldefense.com/v3/__https://github.com/biocore/emperor/blob/b625a5403ce9ad0ef3a84c6bc0b68b33e97888a4/tests/javascript_tests/test_color_view_controller.js*L348-L471__;Iw!!Mih3wA!VeKZkpvzEILHoclhWPbUFi5mPuFp3ezfo9QqGvYAozXbwgUZKc2TQB04LtHDaqc$ ), this error does not seem to be present: the color map being tested is Blues, and the minimum and maximum colors (`#f7fbff` and `#08306b`) match up with the extrema for this color map (going off what `chroma.scale('blues')(0)` and `chroma.scale('blues')(1)` return). I think the problematic part of code is within this block: https://urldefense.com/v3/__https://github.com/biocore/emperor/blob/b625a5403ce9ad0ef3a84c6bc0b68b33e97888a4/emperor/support_files/js/color-view-controller.js*L426-L439__;Iw!!Mih3wA!VeKZkpvzEILHoclhWPbUFi5mPuFp3ezfo9QqGvYAozXbwgUZKc2TQB04MQeUhrk$ The first for loop seems suspicious to me. I'm not completely sure yet what's wrong with this code, but it looks like there might be precision issues with the `s <= max` check: since the data used for the test that failed had a `step` of `(4 - 0) / 100 = 0.04`, while the data used for the test that succeeded had a relatively `step` of `(50 - 0) / 100 = 0.5`. I think it would make the most sense to just remove some of this code and use [`chroma.scale.colors()`](https://urldefense.com/v3/__https://gka.github.io/chroma.js/*scale-colors__;Iw!!Mih3wA!VeKZkpvzEILHoclhWPbUFi5mPuFp3ezfo9QqGvYAozXbwgUZKc2TQB04hp6Uo4k$ ) to do the work for us: ```js var stopColors = interpolator.colors(101); // Now we can construct gradientSVG just like we did before, with the final color (corresponding // to the maximum value) being placed at 100% ``` Can put together a PR for this tomorrow if you'd like. This impacts Empress as well, since the gradient SVG code there was adapted from this code. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://urldefense.com/v3/__https://github.com/biocore/emperor/issues/788__;!!Mih3wA!VeKZkpvzEILHoclhWPbUFi5mPuFp3ezfo9QqGvYAozXbwgUZKc2TQB04BH9AxT4$