Adding discrete seeds to TSP solvers

[supersimple33]

I have made it so that the local_search and simulated annealing will take in a python random.Random pre seeded RNG which can create the needed numbers for making perturbations or for other use cases in each solver.

Addresses #41 and #47

[supersimple33]

I will add the docs part when I get a chance and then ready this for review other than that any thoughts on this implementation

[supersimple33]

Ok, this is ready for review now, I have added the docs.

[supersimple33]

@fillipe-gsm Ready for review whenever you get a chance

[fillipe-gsm]

Hey, @supersimple33 , thanks for the initiative with this PR

Sorry for the late review, I was on vacation and away from my computer.

So, I see this pull request is doing two things: adding a random generator object and a max_iterations arguments.

I personally don't plan on supporting the latter as stopping criterion at all. In my opinion, they are not as intuitive as maximum time, as the user would need to know the internals of the code to understand what an iteration is. Also, this adds more code, so more complexity that needs to be maintained, so I am reluctant on adding more stuff unless absolutely necessary.

So, if you plan on proceeding with the welcoming suggestion of adding a random number generator as argument, I would like you to do the following:

• address the issues that mypy raised. You should see them in the "Checks" panel
• there are no tests in this PR, so can you add some? Here are some ideas for them:
  • pass the same rng to each solver twice and ensure they return the same results
  • mock rng and ensure it got called

Feel free to ask for help if you'd like. And thanks again for your time.

[supersimple33]

Regarding max_iterations you already support it in record_to_record I was just pulling other solving algorithms in line with that spec.

[supersimple33]

@fillipe-gsm Ok I have added tests to the pr following what you have specified. I have also fixed all the mypy warnings it was mostly just missing a few type unions.

[fillipe-gsm]

Regarding max_iterations you already support it in record_to_record I was just pulling other solving algorithms in line with that spec.

I understand your answer, but here are three more reasons why I prefer you to drop the max_iterations argument:

• the record_to_record method is an exception as it uses another heuristic as a base, and, differently from the rest, it has no good appropriate stopping condition
• its concept of iterations is totally different: each iteration runs another full blown heuristic (Lin Kernighan in this case), so 10 iterations in record_to_record are definitely not the same as 10 iterations of local search or simulated annealing. This would just complicate its usage by the user
• it at least has a rule of thumb of appropriate number of iterations (number of nodes in the problem).

Thus, for the other methods apart from the record_to_record, the max_iterations don't add much value but add more code and more effort to maintain this small library.

[supersimple33]

@fillipe-gsm Ok got it. I have removed the max_iterations argument PR should be good to go now.

[supersimple33]

@fillipe-gsm

[supersimple33]

@fillipe-gsm