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

Add information about timeout #33

Open
MalteT opened this issue Dec 16, 2020 · 6 comments
Open

Add information about timeout #33

MalteT opened this issue Dec 16, 2020 · 6 comments

Comments

@MalteT
Copy link
Contributor

MalteT commented Dec 16, 2020

I'm trying to use the wait function on SolveHandle, but can't figure out what unit the timeout is expected to have. Additional information about the parameter might be nice. Alternatively, timeout's type could be changed to std::time::Duration, but that would mean an api change.

clingo-rs/src/lib.rs

Lines 4783 to 4795 in 7cd4f8b

/// Wait for the specified amount of time to check if the next result is ready.
///
/// If the time is set to zero, this function can be used to poll if the search is still active.
/// If the time is negative, the function blocks until the search is finished.
///
/// # Arguments
///
/// * `timeout` - the maximum time to wait
pub fn wait(&mut self, timeout: f64) -> bool {
let mut result = false;
unsafe { clingo_solve_handle_wait(self.theref, timeout, &mut result) }
result
}

Thanks!

(With some information, I could assemble a PR for either solution, if that helps)

@sthiele
Copy link
Member

sthiele commented Dec 17, 2020

Thanks!
To be honest I don't know the unit.
That's the corresponding comment in the clingo C-API:

https://github.com/potassco/clingo/blob/cc79684590215a6b8ae795f6c7983e438d949e28/libclingo/clingo.h#L2227-L2235

@rkaminsk Can you answer this.

I think then implementing Duration should be easy.

@sthiele
Copy link
Member

sthiele commented Dec 17, 2020

Python API says it's seconds https://potassco.org/clingo/python-api/current/

@rkaminsk
Copy link
Member

Yes, it is seconds.

@MalteT
Copy link
Contributor Author

MalteT commented Dec 17, 2020

Thanks for the quick response!

The Duration impl should be easy. I'm willing to implement that. I was mostly concerned about the api change, but if you're okay with that, there should be no drawbacks.

@sthiele
Copy link
Member

sthiele commented Dec 17, 2020

That would be awesome. Momentarily the API is far from stable. I'm looking forward to your PR!

@MalteT
Copy link
Contributor Author

MalteT commented Dec 17, 2020

This is quite unrelated, but I encountered another 'problem' where I tried to use wait without specifying SolveMode::ASYNC when calling Control::solve.

terminate called after throwing an instance of 'std::logic_error'
  what():  Timed wait not supported!
[1]    3125834 abort (core dumped)  cargo run

Am I right in assuming, that SolveHandle::wait should not be called when solving synchronously? If so, we should probably have a note about it or change the signature to this:

pub fn wait(&mut self, timeout: Duration) -> Result<bool, ClingoError>

I wouldn't know how to catch the error, though..

One simple solution would be to add a member solve_mode: SolveMode to SolveHandle and do a simple check, to see if the solving is done async. We could also introduce an AsyncSolveHandle but that would require more work.

Let me know, if that's even accurate and what you think about such a change.

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

No branches or pull requests

3 participants