#16361: OA(7,66), OA(7,68), OA(8,69), OA(7,74) and OA(8,76)
-------------------------------------+-------------------------------------
Reporter: ncohen | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.3
Component: combinatorial | Resolution:
designs | Merged in:
Keywords: | Reviewers:
Authors: Nathann Cohen | Work issues:
Report Upstream: N/A | Commit:
Branch: u/ncohen/16361 | 20ef21682cfe8b6532e2e2621b88d921236528f2
Dependencies: #16356 | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by ncohen):
Hello !
> `incomplete_orthogonal_array`:
>
> - Under the condition `x <= 3 and n > k-1 and k >= 3` it should return
`orthogonal_array(k,n,existence=True)` and not `True`.
+1
> - If `k=n+1` then we have a projective planes and all blocks
intersect... so there is no `OA(n+1,n) - 2.OA(n+1,1)`. I added the
appropriate test in `incomplete_orthogonal_array`.
See [1]
> `OA_from_PBD`:
>
> - it makes more sense to use the function `incomplete_orthogonal_array`
since it is here!!
+1
> - I changed the end of the if/elif to forward the non-existence (and
also avoid doctest error because of #16388)
See below
> - I changed the doctest (which was just a copy of the one in
`database.py`!!) to be something non-trivial
+1
> see the changes on the branch `public/16361` (where #16461 is merged). I
was able to build the doc and all test pass...
>
> Do you have further remarks? Otherwise it deserves a positive review.
Well... Actually, I think that you should not intercept all exceptions in
order to return specific error messages... I believe that you should let
the subfunction raise its own exception. That's going too far I think
`:-/`
If the user requests a construction that requires other construction, let
the other constructions raise an exception ! The user will see a message
like "Impossible to build a OA(....)", meaning that the function needed
one and it did not exist.
{{{
+ for i in K:
+ if incomplete_orthogonal_array(k,i,(1,)*i,existence=True) is
False:
+ raise EmptySetError("The construction is impossible since
there is no OA({},{})-{}.OA({},1)".format(k,i,i,k))
+ elif incomplete_orthogonal_array(k,i,(1,)*i,existence=True) is
Unknown:
+ raise NotImplementedError("I was not able to build an
OA({},{})-{}.OA({},1)".format(k,i,i,k))
+ else:
+ OAs[i] = incomplete_orthogonal_array(k,i,(1,)*i)
}}}
In the first two situations, you return manually the exception that should
be raised by incomplete_orthogonal_array. Why do it again manually ? Why
not just call it ?
Same here
{{{
+ elif orthogonal_array(k,n,existence=existence) is False:
+ if existence:
+ return False
+ raise EmptySetError("There is no OA({},{}) and hence no
OA({},{})-{}.OA({},1)".format(k,n,k,n,k))
+
+ elif orthogonal_array(k,n,existence=existence) is Unknown:
+ if existence:
+ return Unknown
+ raise NotImplementedError("I do not know how to build this
OA({},{})-{}.OA({},1)".format(k,n,k))
+
else:
- return orthogonal_array(k,n,existence=existence)
+ raise RuntimeError("This should not happen!")
}}}
The line you removed was doing the very same job, i.e. forwarding the non-
existence results ! All you did was rewrite the exception, and the user
sees :
"There is no OA(k,n) and this no OA(k,n)-xOA(k,1)"
instead of
"There is no OA(k,n)"
I believe that you should let the subfunction raise its exception. What do
you think ?
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/16361#comment:10>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
