setSortKey does not create an error message when key is too large

[wipfli]

I use the setSortKey to only keep text labels for the largest places in a given tile or subpart of a tile.

My processFeatures function looks like this:

  @Override
  public void processFeature(SourceFeature sourceFeature, FeatureCollector features) {
    if (sourceFeature.isPoint() && sourceFeature.hasTag("wikidata") && sourceFeature.hasTag("name") && (sourceFeature.hasTag("place") || sourceFeature.hasTag("natural") || sourceFeature.hasTag("waterway", "waterfall")))
    {
      var feature = features.point("qrank");
      feature
        .setZoomRange(0, 14)
        .setSortKey(-getQRank(sourceFeature.getTag("wikidata")))
        .setPointLabelGridSizeAndLimit(
          12, // only limit at z_ and below
          128, // break the tile up into _x_ px squares
          5 // any only keep the _ nodes with lowest sort-key in each _px square
        )
        .setBufferPixelOverrides(ZoomFunction.maxZoom(12, 128))
        .setAttr("@qrank", getQRank(sourceFeature.getTag("wikidata")));
      for (var entry : sourceFeature.tags().entrySet()) {
        feature.setAttr(entry.getKey(), entry.getValue());
      }
    }
  }

This works really well in Switzerland where the QRanks are I think all below 2.4M or so. But for larger cities, the label gets dropped. For example, Milano is not included in the map:

https://wipfli.github.io/swiss-map/#map=8.78/45.5971/9.3084

I think this is because the QRank of Milano is 4402075 while the maximum allowed key is int SORT_KEY_MAX = 4194303.

From the source code, I would have expected to get an error message when I give a too large or a too small sort key, but I did not get any error message. Is this expected?

I think this is the relevant part of Planetiler:

    public Feature setSortKey(int sortKey) {
      assert sortKey >= FeatureGroup.SORT_KEY_MIN && sortKey <= FeatureGroup.SORT_KEY_MAX : "Sort key " + sortKey +
        " outside of allowed range [" + FeatureGroup.SORT_KEY_MIN + ", " + FeatureGroup.SORT_KEY_MAX + "]";
      this.sortKey = sortKey;
      return this;
    }

[erik]

I've run into this as well. If there's not already some reference in the documentation (if there is I've missed it) maybe mentioning somewhere that -ea should be given to the JVM is a good idea.

[wipfli]

What is -ea?

[wipfli]

Thanks for the hint...

[erik]

By default, assertions are disabled, -ea ("enable assertions") turns them back on, and would cause all the assert statements to throw exceptions during runtime. https://www.oreilly.com/library/view/introduction-to-jvm/9781787127944/f5b7a6d8-4e0f-41a0-b9c9-0651720472d4.xhtml

[wipfli]

Thanks

[msbarry]

yeah I use assert here so it throws when run from tests but has no runtime overhead. The original intended workflow here would be to have a unit test that exposes the issue (or at least run once with -ea to test), but do you think always logging a warning at runtime would be better?

[wipfli]

Probably it is enough to highlight the -ea flag in the readme. Should I make a pull request?