Leaflet.heat intensity is between 0.0 and 1.0

[trevan]

My heatmaps were looking really strange and after debugging, I discovered that Leaflet.heat expects the intensity to be between 0.0. and 1.0. https://github.com/spalger/Leaflet.heat/blob/gh-pages/src/HeatLayer.js#L129 is where it defaults the max to 1 (Kibana isn't passing in a max value) and https://github.com/spalger/Leaflet.heat/blob/gh-pages/src/HeatLayer.js#L173 makes sure it never goes over the max value.

[elasticsearch-release]

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

[rashidkpc]

jenkins, test it

[epixa]

This needs to be rebased on master or have master merged into it.

[trevan]

I rebased

[epixa]

jenkins, test it

[trevan]

What needs to be done for this to go in? Or is #6014 going to go in instead?

[elasticsearch-release]

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

[epixa]

jenkins, test it

[w33ble]

I've been playing with this for a while now, and I don't think either value is correct. Granted I'm playing with random data, but the output is easy enough to understand.

Using the old code, I see this:

With the changes in this PR, I see this very similar output:

However, I think these are actually both wrong. When I hover over the heat areas and actually look at the values, the values around St. Louis are much larger then everywhere else.

If I just use the value directly for heatIntensity - the same as happens when heatNormalizeData is disabled (an option we removed at some point) - I see something closer to what I believe is the correct output:

Interestingly enough, if I pass in the min and max values to match the data it's trying to represent, it ends up looking more like the first 2 images.

Given all this, I need to spend some more time digging in to how the min and max options work, and what they are actually supposed to do so I can figure out which version of the chart is correct.

[trevan]

The reason it looks pretty much identical at the top level is because the leaflet.heat module uses the current zoomLevel to calculate the intensity (https://github.com/spalger/Leaflet.heat/blob/gh-pages/src/HeatLayer.js#L131 and line 149). If the current zoomLevel and the "maximum zoom" attribute are the same, then it looks a lot different.

Here's an example of zooming in with three different code bases: current code, using the value directly, or using this new code plus changing the maximum zoom to equal the zoomLevel.
Current Code:

Value Directly:

New Code:

With the current code, you don't start to differentiate anything till you get much farther in. That is because as you get closer to the "maximum zoom", the zoom factor of the intensity decreases and so the value can finally be seen.
With using the value directly (as you did), you can see items differently immediately but as you zoom in, the intensity color changes. This is also because of the zoom factor. And it only works because the value that my data has is large enough to overcome the zoom factor. If all you data was less than 100, it would look like the first example.
With the new code (and updating the zoom factor), the intensity color never changes.

I'm not sure if the zoom factor should change automatically but I'm open to adding that to this pull request.

[trevan]

You might have noticed that I didn't try the current code with the auto changing zoom factor. The reason is that it just looks red. The direct value is just as bad. Here's an image of the current code at zoom 2 with the "maximum zoom" set to 2.

[w33ble]

I see what you're saying, and your code does seem more correct. I think the original * 100 bit in there was probably a hack just to get the heat colors to show up better, which isn't the right way to solve that anyway.

I want to dig in to #6014 some more before I merge this, but I think these changes are very simple and reasonable, and produce better results. Should get this in soon, sorry for the delay on it.

[w33ble]

#6014 does in fact fix the same issue, but it also adds extra functionality and needs a little more work. I see no reason not to merge this in the meantime. LGTM!