Hamiltonian path finding function and associated tests#889
Open
ap4108735 wants to merge 22 commits intodigraphs:mainfrom
Open
Hamiltonian path finding function and associated tests#889ap4108735 wants to merge 22 commits intodigraphs:mainfrom
ap4108735 wants to merge 22 commits intodigraphs:mainfrom
Conversation
| return true; | ||
| end); | ||
|
|
||
| InstallMethod(TestHamiltonianPath, "for a digraph", [IsDigraph], |
Member
There was a problem hiding this comment.
This would be more consistent with elsewhere in the Digraphs package if it was:
Suggested change
| InstallMethod(TestHamiltonianPath, "for a digraph", [IsDigraph], | |
| InstallMethod(IsHamiltonianPath, "for a digraph", [IsDigraph, IsList], |
i.e. returns true if the 2nd argument is a hamiltonian path in the 1st argument.
| P := HamiltonianPath(D); | ||
|
|
||
| if P = fail then | ||
| Print("HamiltonianPath returned fail.\n"); |
Member
There was a problem hiding this comment.
This should be an info statement
|
|
||
| # 2. Check if path has no repeated vertices | ||
| if not IsDuplicateFreeList(P) then | ||
| Print("FAIL: Path repeats vertices.\n"); |
Member
There was a problem hiding this comment.
Same as above should be an info statement
|
|
||
| # 3. Check if path length equals number of vertices | ||
| if Length(P) <> DigraphNrVertices(D) then | ||
| Print("FAIL: Path length is ", Length(P), |
| return false; | ||
| fi; | ||
|
|
||
| Print("SUCCESS: Path is a valid Hamiltonian path.\n"); |
Member
There was a problem hiding this comment.
And this should probably be removed.
Member
There was a problem hiding this comment.
This was committed by accident and should removed.
Member
There was a problem hiding this comment.
Remove, unless required for something, but I can't see how it could be.
| # trivial cases | ||
| if v <= 1 then | ||
| return DigraphVertices(D); | ||
| elif v < 256 then |
Member
There was a problem hiding this comment.
Magic number, comment on reason for choice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have added a function to find the more general notion of a Hamiltonian path, to be used alongside the existing Hamiltonian cycle function. I have also added an associated testing function. At the moment, for large cases the function is not very efficient so I will continue to look into some possible fixes for that.