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

Sizing of scaled_circles markers does not work for all aggregation types #8001

Closed
thomasneirynck opened this issue Aug 15, 2016 · 5 comments
Closed
Labels
bug Fixes for quality problems that affect the customer experience [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Coordinate Map

Comments

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Aug 15, 2016

This spun off from PR #7915. Isolating this in ticket, to improve trackability and not clutter this with unrelated changes.

Determining the radius size for circles in the "scaled markers" visualization:

  • relies on magic numbers to force size to an acceptable range
  • forces the values to be positive. For aggregations like SUM, AVERAGE,... it is quite possible that the aggregation yields values in a range from negative to positive.
  • maps 0-values to 0-radii. This would make sense for count aggregations, but ES already omits these anyway. It is perfectly acceptable for SUM, AVERAGE,... to have a meaningful 0-value that should still be visible on the map.
@thomasneirynck
Copy link
Contributor Author

thomasneirynck commented Aug 15, 2016

Since ES 2.0, the min_doc_count for aggregations is set to 0. This means that also for count-aggregations, ES would return a 0-valued bucket.

Dealing with circle size for count and max/min/avg/median aggregations is a similar problem as #6245.

@thomasneirynck thomasneirynck changed the title scaled_circles markers implementation scaled_circles markers implementation does not seem to work for all aggregation types Aug 15, 2016
@thomasneirynck thomasneirynck changed the title scaled_circles markers implementation does not seem to work for all aggregation types Sizing of scaled_circles markers does not work for all aggregation types Aug 16, 2016
@thomasneirynck
Copy link
Contributor Author

thomasneirynck commented Aug 24, 2016

In the #7915, one issue was that the new approach did not avoid overlap. Note that this is also an issue with the older approach (below screenshot is from 4.5.4)

image

The core problem here is that the height of a geohash grid is not consistent for a single zoom level. They become taller the closer you go to the poles. Also, depending on the precision-level, they will be longer/shorter than the width.

@LeeDr LeeDr added bug Fixes for quality problems that affect the customer experience P2 labels Sep 9, 2016
@tbragin tbragin added the Feature:Visualizations Generic visualization features (in case no more specific feature label is available) label Nov 15, 2016
@JacobBrandt
Copy link
Contributor

The objective seems to not have overlapping circles like shaded circles. In addition we want to have varied circle sizes based on the value of the feature. So is there any objection to using the same code from ShadedCircleMarker for the initial max size and then using a linear scale to determine that features radius?

Something like the code below. I also don't like when the circle sizes get really small so I made the linear scale fit within max/3 to max. That's obviously a personal preference so you could just skip that step and do feature.properties.value / this.geoJson.properties.allmax.

function ScaledCircleMarker(map, geoJson, params) {
  let self = this;
  ScaledCircleMarker.Super.apply(this, arguments);

  // multiplier to reduce size of all circles
  let scaleFactor = 0.8;

  this._createMarkerGroup({
    pointToLayer: function (feature, latlng) {
      let scaledRadius = self._geohashMinDistance(feature) * scaleFactor;
      return L.circle(latlng, scaledRadius);
    }
  });
}

ScaledCircleMarker.prototype._geohashMinDistance = function (feature) {
  let centerPoint = _.get(feature, 'properties.center');
  let geohashRect = _.get(feature, 'properties.rectangle');

  // centerPoint is an array of [lat, lng]
  // geohashRect is the 4 corners of the geoHash rectangle
  //   an array that starts at the southwest corner and proceeds
  //   clockwise, each value being an array of [lat, lng]

  // center lat and southeast lng
  let east   = L.latLng([centerPoint[0], geohashRect[2][1]]);
  // southwest lat and center lng
  let north  = L.latLng([geohashRect[3][0], centerPoint[1]]);

  // get latLng of geohash center point
  let center = L.latLng([centerPoint[0], centerPoint[1]]);

  // get smallest radius at center of geohash grid rectangle
  let eastRadius  = Math.floor(center.distanceTo(east));
  let northRadius = Math.floor(center.distanceTo(north));
  let radius = _.min([eastRadius, northRadius]);

  let orgMin = this.geoJson.properties.allmin;
  let orgMax = this.geoJson.properties.allmax;
  // Don't let the circle size get any smaller than one-third the max size
  let min = orgMax / 3;
  let max = orgMax;
  let value = this._scaleValueBetween(feature.properties.value, min, max, orgMin, orgMax);
  return radius * (value / orgMax);
};

ScaledCircleMarker.prototype._scaleValueBetween = function(value, min, max, orgMin, orgMax) {
  return ((max-min)*(value-orgMin))/(orgMax-orgMin) + min;
}

@JacobBrandt
Copy link
Contributor

For anyone interested my solution worked well for me. I changed what I posted before to use L.circleMarker because L.circle was rounding. This helped solve a problem we were having with the circles overlapping each other.

@tbragin tbragin added P4 and removed P2 labels Feb 9, 2017
@epixa epixa removed the P4 label Apr 25, 2017
@alexfrancoeur alexfrancoeur added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation and removed [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation labels Feb 6, 2018
@timroes timroes removed the Feature:Visualizations Generic visualization features (in case no more specific feature label is available) label Apr 11, 2018
@thomasneirynck
Copy link
Contributor Author

Closing this in favor of #12672.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Coordinate Map
Projects
None yet
Development

No branches or pull requests

7 participants