> I posted below the current log of the patch. Altogether, I am > essentially done, except for looking at the antichains optimizations, > and a couple issues to be discussed now:
it looks nice, thanks! > - Currently P.hasse_diagram() returns a graph G whose nodes are the > elements of P (wrapped as elements of P; so one needs to do > x.element to actually get the original element). This graph is still > in the HasseDiagram class. The issue is that HasseDiagram's expect > their vertices to be 0,1,...,n-1, so most of the extra methods just > break. > > Option 1: P.hasse_diagram() returns a plain digraph > > Option 2: P.hasse_diagram() returns the internal HasseDiagram, with > vertices labelled 0,1,...,n > What do you prefer? I personally vote for 2; as a user, that's what > I would expect. I'd also vote for 2 > Another question is whether we want the vertices P.hasse_diagram() > to be wrapped or not. And if we are unhappy with the situation, is > still time to change this? I think wrapping them is fine in the hasse diagram, I think usually people should use the poset itself and not the hasse diagram. To be honest, I don't even see a reason for having a hasse diagram at all, why not doing everything in poset, and taking the hasse diagram only for printing? > - There are many methods that are duplicated, depending on whether we > go up or down (order_ideal / order_filter, ...). It would make sense > to factor out the code, by using an option like: > > sage: P.order_ideal(direction="up") > > (of course keeping P.order_filter as a backward compatibility alias) I would keep order_idael and order_filter, I have the impression, everything else just causes misunderstandings. > I have already used such an option for some of the new methods. What > would be good names for this option and its possible values? > > sage: P.order_ideal(ideal = 'up') # resp. 'down' > sage: P.order_ideal(direction = 'ideal') # resp. 'filter' > > - Do we want panyuschev_complement to accept such an option? > If yes, should the option describe the direction of the ideal or of > its complement? The panyushev_complement is an operation on antichains, so I would prefer not to have such an option. Of course, one can also make it an operation on order_ideals or on order_filters, but I don't see a reason to do so... > - Should methods like order_ideal, order_ideal_generators, > panyushev_complement return set's, tuple's, list's, or Set's? > Currently it's some sort of a mix. I could be convinced to use Sets, > but don't have a strong opinion. I also had this problem already, I would either vote for set's or Set's. As set is faster, that's maybe the right thing to do. > - Does anyone care if the hash function for posets changes? Otherwise > we could just use the default implementation provided by > UniqueRepresentation. That's fine for me. > - Poset elements now have unique representation. This noticably > reduces the overhead of conversion from internal vertices and > elements. In the following an example posted by Christian on > sage-combinat-devel, P.antichains() used to take more than 6s before > the patch: Very nice, thanks! Best, Christian -- 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.
