Skip to content

Conversation

@cqc-alec
Copy link
Contributor

@cqc-alec cqc-alec commented Jul 6, 2021

Currently the only way to time out a search is from the user-provided callback, but the callback is only called when a match is found. This PR provides a way to set a timeout on the search irrespective of whether or when a match is found.

@cqc-alec
Copy link
Contributor Author

cqc-alec commented Jul 6, 2021

Apologies, I have obviously failed to set up linkage with boost::chrono. How to do that?

@jeremy-murphy
Copy link
Contributor

I'm personally not really keen on this idea because it's an optional feature that is orthogonal to the purpose of the algorithm itself, I mean, users could request a timeout parameter on any algorithm. What's wrong the user just using a timer feature from their operating system or from the standard library's threads?

@cqc-alec
Copy link
Contributor Author

cqc-alec commented Dec 5, 2021

I'm personally not really keen on this idea because it's an optional feature that is orthogonal to the purpose of the algorithm itself, I mean, users could request a timeout parameter on any algorithm. What's wrong the user just using a timer feature from their operating system or from the standard library's threads?

I did consider running the algorithm in a thread but the problem is that there is no way (without modifying the boost code in some way) to force it to terminate after a certain time. (The callback is no help because it may be called infrequently or not at all.)

Maybe Boost.Process could help, though I suspect this would necessitate a lot of extra complexity.

I do agree with your point though. Probably we should not really be using this algorithm in cases where very long searches are likely to be an issue. (Our use case involves attempting to solve a problem, then if no solution is found in a certain time making the problem slightly easier, and so on until a solution is found.)

@jeremy-murphy
Copy link
Contributor

Hmmm, right. Yeah, I can certainly see the use case for a timeout.

As you mentioned, the callback is not called frequently enough, so maybe the answer is: another callback! No, seriously. The search algorithms, for example, have a visitor with callbacks for every step of the algorithm. We could have something similar here so that users can terminate their search after searching n vertices, or in your case, n seconds, etc.

@cqc-alec
Copy link
Contributor Author

cqc-alec commented Dec 9, 2021

Hmmm, right. Yeah, I can certainly see the use case for a timeout.

As you mentioned, the callback is not called frequently enough, so maybe the answer is: another callback! No, seriously. The search algorithms, for example, have a visitor with callbacks for every step of the algorithm. We could have something similar here so that users can terminate their search after searching n vertices, or in your case, n seconds, etc.

I like the idea of another callback! That also means we don't have to depend on boost::chrono. I will work on implementing this.

@jeremy-murphy
Copy link
Contributor

Thanks, much appreciated. Please feel free to discuss your design ideas here before doing a full implementation. Adding a new callback could be a delicate operation and we want to make sure it fits with the existing design philosophy.

@cqc-alec
Copy link
Contributor Author

Closing this PR. Will open another one for the callback idea.

@jeremy-murphy
Copy link
Contributor

Closing this PR. Will open another one for the callback idea.

OK, sounds good.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants