On Thu, Nov 29, 2012 at 09:22:57AM -0800, Jesus Torrado wrote:
> Sorry, I missunderstood the question. Exactly, what I was doing is the
> following: I have all the roots of E8, apply a projection condition and,
> with some code, identify from the surviving roots the irreducible Cartan
> types, returning a list of them. Some times (e.g. trivial projection
> condition), the list has just one member (in the example, [["E",8]]). In
> order to test my identification of the Cartan types, I would find the
> simple roots in the surviving root system, compute the Cartan matrix and
> compare it with the one from CartanType[<result>], where <result> is the
> list of types identified. But things like
> CartanType([["E",8]]).cartan_matrix() just fail, as we know. What I did is
> of course fix it on my side, using the same two lines as in the patch.
> Anyway, as it is a mathematically wrong answer, I decided to report a bug.
Thanks much for the detailed use case!
> So, let me dare to say that we disagree with the first two
> possibilities and look at the last two:
Sounds good. I was also uncomfortable with them, and your use case
definitely rule them out.
> - Return the underlying CartanType A (that's what the current patch
> implements). That's mathematically correct. However, having a non
> uniform behavior in corner cases typically forces any caller code to
> handle explicitly the special cases. For example, in the case at
> hand, the nodes of the Dynkin diagram would not get relabeled; also
> the ambient space would not have an inject_weights method, etc.
>
> Maybe it is a naA-ve question, but why would we want the relabelling or
> the weight injection if we actually have an irreducible type?
It's a good question. So let's decide for now that CartanType([A])
returns A. Suppose you want to code a function f that builds some list
L=[C1,...,Cn] of Cartan types. Then, it builds the ambient space for
C, and does some calculations that requires to build some weights for
C from weights from the Ci's:
C = CartanType(L)
A = C.ambient_space()
c = < some list of weights for C1, ... Cn respectively >
weight = A.inject_weights( c )
Problem: this code will break whenever L will turn out to have a
single element. Therefore you need to pollute the code of f with the
handling of a special case:
C = CartanType(L)
A = C.ambient_space()
if len(L) == 1:
weight = c[0]
else:
weight = A.inject_weights( [c1, ..., cn] )
> Btw: if we go in this direction, we should add tests, in particular
> for the corner-corner case: CartanType([[A]]).
>
> I disagree there: this is not a corner-corner case, but plain wrong input,
> which do not fit in any of the examples given in the docstring.
Why wrong? Granted, it's currently unspecified. However, in order to
accept:
CartanType([["A",2], ["A",3]])
one more or less wants to specify that «CartanType accepts as input a
list of objects each of which can be made into a CartanType». And then
this naturally extends recursively to more nested lists of lists.
Well, this also opens another can of worms: would we want the direct
product of cartan types to be associative. I would argue for not, for
similar reasons: (A x B) x C is certainly isomorphic to A x (B x C),
but the isomorphism is not completely trivial due to the potential node
relabelling. However that's another question which we can leave
aside for now.
> Still, it could be easily incorporated by changing the "if" in
> the patch for a "while".
Certainly. I am just saying that, whichever option we choose, there
should be a test illustrating the choice.
> - Rename type_reducible to type_direct_product, and fix is_irreducible
> and friends to handle the special case properly. But it's a bit more
> work. I guess it would be enough to remove the definition of
> is_irreducible, and use a trick in the initialization like for
> type_relabel, to make the result inherit from CartanType_simple when
> relevant.
>
> That seems to me innecesarily complicated,
Yes, arguably more complicated than adding a single `if`. But still
pretty minimal: renaming a class, removing a method, and doing a
little adjustment in the init. We could even be lazy and not rename
the class, though calling "type_reducible" something that, in some
cases, is not would be the ugly.
> maybe because I am missing your point, maybe because I don't see
> "my" corner case as an special case, but as a normal case which
> is handled wrongly. Could you, please, elaborate on this point a
> little more?
Again: corner cases are normal cases in the sense that they certainly
should be handled correctly. And as uniformly as possible, so that
callers code need not do a special case.
What do you think?
> Whichever option is chosen, I volunteer, as long as there are no
> hard time constraints:
Great, thanks! I appreciate that.
> I certainly enjoy this, but the last year of a PhD is maybe not
> the best moment to start working too much in anything else than
> one's thesis :(
Definitely!!!
Cheers,
Nicolas
--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/
--
You received this message because you are subscribed to the Google Groups
"sage-combinat-devel" group.
To post to this group, send email to sage-combinat-devel@googlegroups.com.
To unsubscribe from this group, send email to
sage-combinat-devel+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/sage-combinat-devel?hl=en.
