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

mileage_by_region #212

Open
1 task
yakra opened this issue Feb 28, 2022 · 1 comment
Open
1 task

mileage_by_region #212

yakra opened this issue Feb 28, 2022 · 1 comment

Comments

@yakra
Copy link
Owner

yakra commented Feb 28, 2022

search for mileage_by_region

  • 'T': Travelerlist::active_preview_mileage_by_region and Travelerlist::active_only_mileage_by_region can become a single std::unordered_map<Region*, double[2]>
    • fewer lookups
    • fewer exceptions
    • fewer creations
    NO-BUILD -- no noticeable speed increase.

  • 'T': Same deal in Python, using lists or tuples
    NO-BUILD -- ostensible ~0.17-0.25s speedup may just be noise. Stay in line with the C++ version, with more readable code, and a cleaner commit history. Deleted branch @ ba5d725e84d2ed15940c244b5e17fcb703afd1be.

  • 'G': Same for Python's 3 global active_only_..., active_preview_... & overall_... dicts
    • Nothing to change in C++; these data are members of the Region class.
    LOW PRIORITY -- between 'T' having next to no effect with 4.5x (6.74 travelers per segment * 2/3 the dict lookups) as many lookups, and 'C' having next to no effect going from 1 lookup/segment to 1 lookup/route, I don't expect much here.

  • 'C': Copy the C++ approach to overall stats: Calculate subtotals per Route, then add to regional totals. Results in 1 lookup per Route instead of 1 per HighwaySegment.
    Naah... The difference was much less than anticipated:

    box before after
    lab1 12.7244 12.4266
    lab5 11.9216 11.2950

    (deleted stash @ bc351591af3159b764b8e07f10be7dfec4dae2bd / sulogs @ 2022-02-28.py/T3.tar.lzma)
    Too little IMO to justify the... distasteful, rounding needed to get t.active_systems_clinched & t.preview_systems_clinched to come out right. Even if the same is done (via a float conversion) in C++.
    Maybe? C++: Look into constructing & iterating thru per-region route lists. No need for region->mtx(even though speed already rounds to 0). Should be able to lose the float conversion that way. Low priority.

@yakra
Copy link
Owner Author

yakra commented Mar 21, 2022

I forget what exactly happened here. IIRC Python showed no improvement. If C++ did at all, it was slight -- and was it overshadowed by TravelMapping#504?
Spreadsheet notes aren't clear; sulogs should be in the late Feb - March 3 2022 ballpark. That's right about when I started versions.txt; does one exist?

I closed TravelMapping#504, then a day later marked this low priority, indicating I wasn't completely done with it.
There are enough other things to tweak nearby, in particular how unordered_maps are handled, that this can wait till afterwards to see if it's any help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant