Checks for `is_separable` are not complete
Closed this issue · 7 comments
Several computational conditions are checked to determine if a state is separable presently in is_separable.py. However, several items are still not covered compared to the IsSeparable function from QETLAB.
This issue covers the additional separability checks from IsSeparable and includes test cases for each condition.
Note: At present, if all other separability checks are not hit, the failsafe is the final separability check imposed by invoking the symmetric extension hierarchy. This will eventually converge and will determine whether the state is separable (however this could be computationally intractable in general, and ideally, hitting as many of the prior cases first is ideal).
With that in mind, not all separability checks from QETLAB need to be ported over, but adding some more of them into toqito would be ideal. This issue could be considered closed then if a subset of the separability checks are ported over.
Hello, I have added a few conditions but it seems that testing those individually is challenging (especially as the condition lies below in the code) since the test cases tend to return a value (True/False) before reaching that point. One could curate the test cases to ensure it returns a value only for the condition to be tested, but I was wondering of some alternatives:
- Do you think it may be plausible to separate out each condition in a separate function? It may then be possible to test a specific condition in the tests. I understand some conditions are only conditionally checked, but perhaps the separate functions need not worry about that and could be handled in the driver code. Do you agree? If not, do you have any suggestions for generating curated test cases?
- I was thinking it might be better to print the reason for why a value is returned along with True/False, so that things are more transparent. This has been done in a few places but can be extended to all places.
Hi @Yash-10 , thank you for your interest in working on this issue.
Indeed there are some test cases that have not been tested yet.
We have an open issue for increasing the test coverage #293. My last attempt to work on this was not that successful because the tests were not moving into a specific part of the code for certain use cases. Vincent's feedback on this was to temporarily add a print line to each rule in the hierarchy to check which code block was being used by is_separable for which unit test.
My plan for finding additional examples (test cases) was to go through the references listed in the docstring of the QETLAB function.
Let's wait for Vincent's feedback on the part where you want to break down the function into smaller functions. I think the comments in between the code are enough as is.
Thank you, @purva-thakre, for your reply! Indeed, designing the test cases might take some effort. I have added references at (almost) all locations where output is returned in #612. Having added the conditions, I do tend to agree that the comments in the code might be good enough since there are many small, conditional checks in the code, and creating a function for each check may not be needed.
Hi @Yash-10
Thanks for taking a stab at this one. And yes, I agree with @purva-thakre that keeping the checks in line is probably more sensible, as you rightfully point out, they should be small enough to not warrant separate functions.
Let us know if there are any additional questions, etc. you have, and thank you again for your contributions thus far!
@vprusso and @purva-thakre thank you for merging the PR! Do you think the issue can be assigned now?
Fixed by #612
I believe @purva-thakre assigned you the issue. Let us know if there's anything else we missed, and thank you again for your contribution, @Yash-10 !