WilliamHPNielsen / limnos

Other
0 stars 0 forks source link

Impose canonical branch order for Trails #11

Open WilliamHPNielsen opened 3 years ago

WilliamHPNielsen commented 3 years ago

It was suggested by @jeppetrost (in the discussion of #8) that we impose a canonical order of Trails branches. I really like that idea and this PR proposes one way of doing it. The idea for the canonical order is that branches are ordered by their distance - measured along the main route - from the beginning of the main route.

If this approach gets approved, I'd like to implement the __eq__ of Trails in this PR too, using the canonical order. Once we have __eq__, adding tests for many things, including the canonical ordering itself, becomes much easier.

I'm marking this PR draft until we agree to keep/discard the ordering proposed here.

@jeppetrost @petterbejo

jeppetrost commented 3 years ago

I like the idea, but is it not possible for two branches to attach to the same point of the main route? (at a four-way corssing) How do we uniquely define the order for those branches?

jeppetrost commented 3 years ago

While my comments on the specific implementation stands, I do like the idea. We finally need to agree on the canonical order of a 4way crossing. It could follow the route, and, say, the "right" path is first. It could also simply be north>south, west>east or something similar. That avoids having to deeal with the direction of the main branch when ordering the sub-branches.

WilliamHPNielsen commented 3 years ago

Okay, let me take a stab at a resolution of the four way crossing issue while keeping the overall idea the same. Stay tuned.

petterbejo commented 3 years ago

My suggestion for the four-way crossing is that on such occurences, we take into consideration the next step of each of the trails and the point with the lowest numbers are first in the canonical order. If, for instance, they branch off from point (9, 9) and one of them has (9,11) as its next point and the other goes to (9, 7), then (9,7) comes first. If they branch off from (9, 9) to (7, 9) and (9, 7), then (7, 9) comes first.

This way, we let Python do the work, and we don't have to bother how to explain Python what is left, right, North, or South.

Apart from the four-way crossing I agree that branches should be ordered according to their position on the solution route.

WilliamHPNielsen commented 3 years ago

Okay, here's an implementation where branches are ordered in a "left before right" fashion. The reason is that I found it appealing to be able to say

self._branches = sorted(self._branches, key=_assign_branch_number)

which requires there to a key function of a single argument. In other words, in order to use sorted, it has to be possible to assign a "number in the line" to a single branch, independently of how many other branches there might be. The way that number is assigned here is simply by saying that a main route of length N has 2*N - 2 slots for branches (-2 for start and end, where no 4-way cross is possible). With an arbitrary choice of "left goes first", one can then uniquely assign each branch a number.

Note that it's not even important whether we consistently choose "left goes first" or sometimes let "right go first". I do think, however, that the does_branch_go_left variable always chooses "left first".

Finally, I would like to add that the testing added here is very rudimentary. We would benefit a lot from using property-based testing; randomly (but with deterministic seeding) generating complex Trails and comparing those.