Infinite loop in NetworkTools.findClosestLinksSorted

[markusstraub]

I just wanted to report that NetworkTools.findClosestLinksSorted produces an infinite loop here: https://github.com/matsim-org/pt2matsim/blob/fdfe40a0d5adb4ed98dff4de13662c60cd74dcbb/src/main/java/org/matsim/pt2matsim/tools/NetworkTools.java#L210 while calling this method on a 1km² test map with meter-based projection.

Unfortunately I can not look for a solution any deeper than finding this copy/paste bug (should be l.getToNode() instead of l.getFromNode() https://github.com/matsim-org/pt2matsim/blob/fdfe40a0d5adb4ed98dff4de13662c60cd74dcbb/src/main/java/org/matsim/pt2matsim/tools/NetworkTools.java#L206

.. but even when fixing this the problem does not go away.

[polettif]

Can you share the test map or an extract with the relevant links? I can't really tell what's going on. Mixing to and from nodes doesn't sound good and definitely looks like a bug, I'm surprised it hasn't come up sooner.

[markusstraub]

I tested my code with this test network: https://github.com/ait-energy/matsim-drs/tree/main/data/floridsdorf But I saw that the NetworkTools.findClosestLinksSorted isn't used in pt2matsim anywhere and also has no test coverage - instead NetworkTools.findClosestLinks is used - so maybe the method is an artifact from development long ago?

[polettif]

But I saw that the NetworkTools.findClosestLinksSorted isn't used in pt2matsim anywhere and also has no test coverage - instead NetworkTools.findClosestLinks is used - so maybe the method is an artifact from development long ago?

Yes, sounds like an artifact. I think I realized that there's no use case to get a list sorted by distance (and link direction) without also providing the distance. NetworkTools.findClosestLinks provides the distance in a SortedMap, LinkCandidateCreatorStandard.findClosestLinks then simply picks both links if they're within the threshold. Though I don't know what your application exactly is.

[markusstraub]

I'm now happily using NetworkTools.findClosestLinks, so maybe just remove the method if it is not needed?

[polettif]

Yes, then it's best to remove it. While we're at it, there are probably some other unused or uncovered methods in CoordTools or NetworkTools worth a closer look.

[marecabo]

Hello @markusstraub @polettif,

As I am preparing a new release, I ~fixed~ "adjusted" the method regarding that copy-paste bug but also deprecated it, so we can remove it in a future release.

That does not solve your issue, so I will not close it, but is that fine for you for now?

[markusstraub]

That's fine for me!