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

not_empty_in makes unchecked assumptions about its input #16345

Open
asmeurer opened this issue Mar 20, 2019 · 3 comments
Open

not_empty_in makes unchecked assumptions about its input #16345

asmeurer opened this issue Mar 20, 2019 · 3 comments
Labels

Comments

@asmeurer
Copy link
Member

I noticed this when doing #16344.

In not_empty_in, there is this code:

if isinstance(finset_intersection, FiniteSet):
finite_set = finset_intersection
_sets = S.Reals
else:
finite_set = finset_intersection.args[1]
_sets = finset_intersection.args[0]
if not isinstance(finite_set, FiniteSet):
raise ValueError('A FiniteSet must be given, not %s: %s' %
(type(finite_set), finite_set))

The code assumes that any set that isn't a finite set is an intersection with two arguments, a finite set and another set. It also assumes that the finite set will be the second argument of the intersection.

If the algorithm can only work in this case, that is fine, but it should check the input to make sure that the object actually is what it thinks it is. Otherwise, it will produce wrong results.

@kangzhiq
Copy link
Contributor

Hello,

I would like to work on this issue.

but it should check the input to make sure that the object actually is what it thinks it is

By saying this, do you mean that we should check if the input finset_intersection has args[1] and arg[0] as finite set and other set?

if not isinstance(finite_set, FiniteSet): 
     raise ValueError('A FiniteSet must be given, not %s: %s' % 
                      (type(finite_set), finite_set))

This if clause is one the checks, isn't-it? Could you please guide me in this?
Thank you in advance!

@asmeurer
Copy link
Member Author

That is one type check, but there are other assumptions, like assuming that finset_intersection is an intersection and assuming it has exactly 2 args. Also, it should be able to handle the case where the finite set is first as well. Intersection sorts the sets by the infimum, so they can be in either order.

@kangzhiq
Copy link
Contributor

@asmeurer OK! I will come back to this issue later, I am preparing the proposal for GSoC 2019.

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

No branches or pull requests

2 participants