Add ordinal representation generator for closed Hamiltonian tours - #13647
Naman-Vasudev wants to merge 7 commits into
Conversation
for more information, see https://pre-commit.ci
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Thanks @Naman-Vasudev — the shrinking-reference ordinal encoding is correct and the doctests demonstrate it well.
Two small things:
reference.index(city)will raise a bareValueErrorif apathcity isn't innodes. Consider validatingset(path) == set(nodes)at the top with an explicit message, so the failure mode is clear.- The module docstring is a bit thinner than its sibling #13645 — adding a reference link and a note about the inverse operation (reconstructing the path from the ordinal vector) would round it out.
Nice pair of contributions.
Refactor ordinal_representation_closed to return an iterator instead of a list. Update examples and main function to reflect the change.
|
@priya-sundaram-dev, please review. Is this code useful for genetic algorithms? |
|
Reviewed locally on 3.12 — the two functional doctests reproduce exactly ( On "is this useful for genetic algorithms?" — yes, but only half of it is here. Ordinal representation is a genuine, well-known GA encoding for the TSP. Its whole point is that an ordinary fixed-point crossover on two ordinal vectors always produces a valid tour (no repair, no PMX/OX bookkeeping). That's exactly why it belongs under But usefulness in a GA needs the round trip, and only the encode direction is implemented. After you cross two ordinal vectors you're left with an ordinal vector that has to be decoded back to a tour. The inverse is symmetric and tiny: def tour_from_ordinal(ordinal: list[int], nodes: list[str]) -> list[str]:
reference = nodes.copy()
return [reference.pop(i - 1) for i in ordinal]I verified Suggestions to make this land as a GA contribution rather than a standalone transform:
With the inverse + a crossover demo I'd be happy to see this merged; as-is it's a correct but one-directional utility. Nice, clean generator style otherwise. |
Added references to Hamiltonian path and problem in docstring.
|
@priya-sundaram-dev, please add a few GitHub suggestions that would enable us to have a mergeable pull request, |
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Two suggestions to get this merge-ready (checks are already green; these are about completeness/house-style).
-
See the inline suggestion below — it adds the inverse
tour_from_ordinalwith a round-trip doctest and switches__main__todoctest.testmod(). -
Naming nit (not a one-click suggestion since it touches the def and every doctest call): the name
ordinal_representation_closedpromises a closed tour, but nothing here wraps the last city back to the first — the logic is identical for an open path. I'd either drop the_closedsuffix, or genuinely close the tour (appendpath[0]) and say so in the docstring. Your call, but the name and the behaviour should agree.
| if __name__ == "__main__": | ||
| sample_path = list("ODGLAHKMBJFCNIE") | ||
| all_nodes = sorted(set(sample_path)) | ||
| print("Ordinal Representation:") | ||
| ordinal_values = ordinal_representation_closed(sample_path, all_nodes) | ||
| print(" ".join(map(str, ordinal_values))) |
There was a problem hiding this comment.
The one thing keeping this from being a self-contained GA contribution is that only the encode direction exists. Adding the inverse makes the round trip testable and swaps the print demo for a doctest.testmod() runner (the house style here):
| if __name__ == "__main__": | |
| sample_path = list("ODGLAHKMBJFCNIE") | |
| all_nodes = sorted(set(sample_path)) | |
| print("Ordinal Representation:") | |
| ordinal_values = ordinal_representation_closed(sample_path, all_nodes) | |
| print(" ".join(map(str, ordinal_values))) | |
| def tour_from_ordinal(ordinal: list[int], nodes: list[str]) -> list[str]: | |
| """ | |
| Decode an ordinal representation back into a tour. | |
| This is the exact inverse of ``ordinal_representation_closed``. It is what | |
| makes ordinal encoding useful in a genetic algorithm: an ordinary one-point | |
| crossover of two ordinal vectors always decodes to a valid tour, with no | |
| repair step needed. | |
| >>> nodes = list("ABCDEFGHIJKL") | |
| >>> path = list("GLADBIKEHJFC") | |
| >>> encoded = list(ordinal_representation_closed(path, nodes)) | |
| >>> tour_from_ordinal(encoded, nodes) == path | |
| True | |
| """ | |
| reference = nodes.copy() | |
| return [reference.pop(index - 1) for index in ordinal] | |
| if __name__ == "__main__": | |
| import doctest | |
| doctest.testmod() |
Describe your change:
This pull request adds a new algorithm that converts a closed Hamiltonian
tour (path) into its ordinal representation based on a fixed reference
order of cities. Each position in the resulting list represents the
1-indexed position of the corresponding city in the reference list,
which shrinks as each city is removed — ensuring a unique and reversible
representation.
Key Features:
ordinal_representation_closed()with descriptive variable namespython -m doctest -vlist[str],int)Checklist: