Concurrent segment detection, part 2

[yakra]

Current:

for h in highway_systems:
 print(".",end="",flush=True)
 for r in h.route_list:
 for s in r.segment_list:
 if s.waypoint1.colocated is not None and s.waypoint2.colocated is not None:
 for w1 in s.waypoint1.colocated:
 if w1.route is not r:
 for w2 in s.waypoint2.colocated:
 if w1.route is w2.route:
 other = w1.route.find_segment_by_waypoints(w1,w2)
 if other is not None:
 if s.concurrent is None:
 s.concurrent = []
 other.concurrent = s.concurrent
 s.concurrent.append(s)
 s.concurrent.append(other)
 concurrencyfile.write("New concurrency [" + str(s) + "][" + str(other) + "] (" + str(len(s.concurrent)) + ")\n")
 else:
 other.concurrent = s.concurrent
 if other not in s.concurrent:
 s.concurrent.append(other)
 #concurrencyfile.write("Added concurrency [" + str(s) + "]-[" + str(other) + "] ("+ str(len(s.concurrent)) + ")\n")
 concurrencyfile.write("Extended concurrency ")
 for x in s.concurrent:
 concurrencyfile.write("[" + str(x) + "]")
 concurrencyfile.write(" (" + str(len(s.concurrent)) + ")\n")
print("!")

Proposed:

for h in highway_systems:
 print(".",end="",flush=True)
 for r in h.route_list:
 for s in r.segment_list:
 if s.concurrent is None and s.waypoint1.colocated is not None and s.waypoint2.colocated is not None:
 for w1 in s.waypoint1.colocated:
 for w2 in s.waypoint2.colocated:
 if w1.route is w2.route:
 found = w1.route.find_segment_by_waypoints(w1,w2)
 if found is not None:
 if s.concurrent is None:
 s.concurrent = []
 found.concurrent = s.concurrent
 s.concurrent.append(found)
 else:
 found.concurrent = s.concurrent
 s.concurrent.append(found)
 #concurrencyfile.write("Added concurrency [" + str(s) + "]-[" + str(found) + "] ("+ str(len(s.concurrent)) + ")\n")
 if len(s.concurrent) == 2:
 concurrencyfile.write("New concurrency ")
 else:
 concurrencyfile.write("Extended concurrency ")
 for x in s.concurrent:
 concurrencyfile.write("[" + str(x) + "]")
 concurrencyfile.write(" (" + str(len(s.concurrent)) + ")\n")
print("!")

1. if s.waypoint1.colocated is not None and s.waypoint2.colocated is not None: ->
   if s.concurrent is None and s.waypoint1.colocated is not None and s.waypoint2.colocated is not None:
   This prevents us from going deeper into the FOR loops and finding concurrencies that are already found. If s.concurrent is not None, it should be fully populated with no need to check further, because:
2. if w1.route is not r: removed: this enables us to find all concurrencies for a given segment on the first pass, including concurrencies on the same route.
3. other renamed to found -- as s itself is now found this way, it's not strictly an "other" any more, is it?
4. s.concurrent.append(s) deleted, as s is now found, and appended that way.
5. if found not in s.concurrent: removed: On the first pass, this is the first time a concurrency will be found, and hence will not be in s.concurrent yet. Future passes are now prevented, making this check unnecessary.
6. New concurrency logging moved out of the if s.concurrent is None: case and moved into the else:, because this would log a single-segment concurrency. There's a new IF/ELSE check to determine whether to log "New concurrency " or "Extended concurrency " (shouldn't be a big performance hit :) ), but this was only really done to maintain compatibility with the existing concurrencies.log format, and make DIFFs human-readable while testing this out.
   This format, listing each route as it's detected & added to a given segment, certainly made sense when first developing the concurrency detecting code, to make sure things were behaving as expected. Now, with everything pretty stable & behaving as expected for some time now, it may make some sense to trim out some fat and simplify the concurrencies.log file by listing only the final concurrency, with all routes for given segment:

for h in highway_systems:
 print(".",end="",flush=True)
 for r in h.route_list:
 for s in r.segment_list:
 if s.concurrent is None and s.waypoint1.colocated is not None and s.waypoint2.colocated is not None:
 for w1 in s.waypoint1.colocated:
 for w2 in s.waypoint2.colocated:
 if w1.route is w2.route:
 found = w1.route.find_segment_by_waypoints(w1,w2)
 if found is not None:
 if s.concurrent is None:
 s.concurrent = []
 found.concurrent = s.concurrent
 s.concurrent.append(found)
 else:
 found.concurrent = s.concurrent
 s.concurrent.append(found)
 concurrencyfile.write("concurrency ")
 for x in s.concurrent:
 concurrencyfile.write("[" + str(x) + "]")
 concurrencyfile.write(" (" + str(len(s.concurrent)) + ")\n")
print("!")

From here, we can see that another possibility is removing the innermost IF/ELSE, and initializing concurrent to an empty list in the HighwaySegment constructor.
I wonder though, whether as opposed to None, this would mean a larger RAM footprint. I'd want to test this out before implementing it.

Between items 2 & 4, we at first see a reduction in efficiency: instead of just doing s.concurrent.append(s), we instead must find_segment_by_waypoints. If we're looking for, say, TX I-10 880 TX/LA, it could take a bit to loop thru the FOR loop in that function. This disadvantage is negated farther down the line, when we no longer need to search for TX US90 I-10(880) TX/LA. Throw a third route or more into the mix, and we have a net savings.

[yakra]

This new concurrency detection method picks up one segment in each of:
http://travelmapping.net/hb/index.php?r=ut.ut190
http://travelmapping.net/hb/index.php?r=ms.tourrd
Like PA US19TrkPit, these routes are concurrent with themselves.
Unlike PA US19TrkPit, these routes are not concurrent with any other routes.

I have no big objections to the way these are plotted, but...
Comments? Thoughts?

[yakra]

FWIW:

ms.tourrd:

• Of 7 travelers on MS TourRd, 6 have travelled the self-concurrent segment. All 6 .list files have a line that explicitly contains both concurrent segments.

ut.ut190:

• 4 travelers have UT UT190 I-215 BriRes in their .lists, BriRes being the old label for UT190_C:
  bobcobb, crosboro7, osu97gp, the_spui_ninja
  As things are now, these four travelers have the concurrent segment counted toward their mileage only once.
• 2 travelers have clinched UT UT190: Based8 & roadguy2.
• norheim hasn't travelled the extension to Guardsman Pass, but has UT UT190 I-215 UT190_D .listed, and thus has the concurrent segment counted twice for mileage.

[yakra]

http://forum.travelmapping.net/index.php?topic=2805

[jteresco]

So the new concurrency would pick up the UT 190 concurrency while the old would not?

[yakra]

Correct.

[jteresco]

Let's try it then.

[yakra]

I want to get #144 squared away before I go ahead with further changes.

That aside, I've been given just enough pause to reconsider... From the forum:

My current thinking:

I tried to think of cases where a user's traveled only one direction, and not the other, and would thus have reason to just want to claim one "direction".
They'd have to leave the "loop" of these routes, and return to the outside world.
UT190: not too easy.
On TourRd, I first saw MelPl and RodDr and thought this was possible.
OTOH, Checking aerial imagery and GMSV, there are actually no connections.
OT3H, sometimes it's wise to anticipate the unanticipated. New roads could be built here, connecting to the outside world. Or a new route could be added to the database in the future, with such a setup.

It may be best to skip my proposed changes to the code (even if it seems better from a programming standpoint), leaving travelers the option of .listing one or both segments.

The latest siteupdate.log shows concurrency detection took 5.6 seconds. Not terrible. :)
Curious though, how much of that this would save. (May try it out anyway just to see that...)
Another approach that might yield middle-of-the-road efficiency: Implementing these changes, then afterward, backing out any HighwaySegment that "is_self_concurrent_only()", removing cases like UT190 & MS TourRd and leaving cases like PA US19TrkPit, which would keep the current .list/augment/stat functionality.

[yakra]

Taking a different approach for #449. Less of a departure from the original code. Should be faster; avoids some calls to find_segment_by_waypoints, the extra if/else at the end, and an extraneous for loop for new concurrencies.