-
Notifications
You must be signed in to change notification settings - Fork 4
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
robustness issues #3
Comments
@soonhokong Yes, this is a known problem. I'll prevent this from terminating the whole shell. On another level, even if you reset the context, you will still get this error. This is what I meant in the earlier email. ctx = Context(qf_nra)
x = Var(ctx,Int,"x")
reset_ctx!(ctx)
x = Var(ctx,Int,"x")
# Error: symbol already declared x (triggered at /home/zenna/repos/dreal3/src/opensmt/egraph/EgraphStore.C, 651) |
@zenna, I see. Could you enumerate examples here for the record? I can fix them one by one and use them as test cases. |
@soonhokong I think any example where you define a variable that has been defined before in a context will cause that error. I'm not sure what kind of different examples you mean. There is another kind of bug with deleting a variable, I'll try to isolate that.
Yes, and maybe return a warning too. But there does not seem to be anything in the API that would allow me to update an existing variable, is there? Actually, generally I think the use of strings as variable names is not ideal, and probably it would be better if they were generated randomly and hidden from the user. |
I like this idea. Let me add a new function in C APIs which does not require string argument.
I can change it to print out an warning to std::cerr. Will that be OK? Or do you want to get some different return value for this case?
For now, we don't have one. But we can add it since it's plausible. Let me know if you need it. |
Assuming that dReal actually requires string variable names, we can do what I proposed in either C or Julia. It might be easier in Julia because dealing with std::cerr is cumbersome.
This may actually be useful, but not so much for the issue we have been describing here. I might open another issue about this. |
OK. I will leave the decision to you then. Just let me know if you think implementing it in C is better..
I agree. |
@soonhokong I've added some checks so that it doesn't fatally error. Any progress on the context resetting problem? |
Also deleting contexts causes malloc errors. See test/optimise.jl for example. |
@zenna, thank you. I'll fix it later today.
Sorry, not yet.. |
@zenna, could you check whether we still have a problem with resetting a context? |
Both problems still persist (at least when using the API from the Julia side)
|
@zenna, thanks for checking things! |
@zenna, it's been a year! Could you check the above two problems persist? If so, could you let me know how to reproduce them? |
Dear @zenna,
While testing the Julia interface, I've found a robustness problem. When I declare a variable twice, it terminates the whole Julia shell:
I think it should return the previously declared variable, possibly update the domain with new information. Do you agree? If so, I can make this change.
Do you see any other robustness issues? Please feel free to enumerate them here, I'll fix them later.
The text was updated successfully, but these errors were encountered: