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
Matrix term ordering #6922
Comments
comment:1
The patch introduces matrix term ordering, but it does not work yet unfortunately.
Please someone who knows better check the patch, and make it work! Simon says: Better wait for a proper implementation in libsingular Cheers, |
comment:2
I don't think we should support the 'matrix()' syntax. The Singular syntax is 'M()' which we should support for compatibility and familiarity. However, adding another string expression does not seem to make sense, because we will be able to simply write |
comment:3
I agree partially. Should we follow the Singular syntax exactly? For short syntax, how about just "m(1,3,1,0)"? I personally think the Singular syntax for term ordering is somewhat cryptic. I think it is better for Sage to support both the string description and
for a square matrix m,
are all supported. Marshall Hampton says: I agree with John that Simon's example:
looks good and reasonably intuitive to me. |
Changed author from Kwankyu Lee to none |
comment:4
Replying to @kwankyu:
Sure, but it would be accepted anyway and passed through to Singular (in an ideal implementation) :)
This description is a mix of Singular syntax and Sage string syntax. I would like to avoid Sage string syntax as much as possible.
I like this best.
Yep, that's what I would be aiming for. |
comment:5
The replaced patch is still in beta stage, due to lack of docstrings. It adds matrix ordering to Sage term order suite, which works fine. It is based on Sage 4.4.2. The reason that I uploaded this premature patch is related with the ticket !#9003 |
comment:6
The replaced patch now works, though only with PolyDict engine. |
Author: Kwankyu Lee |
comment:9
Replying to @kwankyu:
Sorry for being a bit obnoxious about it, but shouldn't this be needs work until it works across the board? At least we should fall back to the |
comment:10
Replying to @malb:
I am sorry that I cannot understand what you mean. Do you mean that matrix term order should work with Singular version before this patch is merged? Now, if a matrix term ordering is selected, then {{PolyDict}} version is used automatically because Singular version does not support matrix term ordering. If someone implements matrix term ordering with Singular version, then I would be happy. I have been waiting for this to be done. As it is not the case, I thought it's not bad to add matrix term ordering support only with {{PolyDict}} version--Singular version may be added later. |
comment:11
Right, I forgot that I implemented to fall back this way :) Okay, my bad then.
We all would be happy. All one needs to do btw. is to fix the constructor.
Why not give it a try yourself? I'd love to help but other commitments prevent me from doing so.
You are right, I stand corrected. |
comment:12
I stand corrected twice. YOU implemented it to fall back not me. I thought the automatic fallback would kick in, but from your patch it looks like it doesn't. |
comment:13
|
comment:14
Oh, one more thing, isn't TermOrder([0,1,2,3]) ambiguous since it could be interpreted as a list of weights? I'd vote to not to allow it for matrix term orders. |
comment:15
Replying to @malb:
I agree.
I know Singular allow matrix term orderings in block order. But this feature is not one of the aims of the current patch. That could be included in a future patch that use Singular version.
I did not agree then. :-) Anyway, I will change it to 'm(...)' |
comment:16
Replying to @malb:
Ok. |
Attachment: trac#6922.patch.gz replaced |
comment:17
The patch applies cleanly and doctests pass. I'm still not happy with the interface: sage: P.<a,b> = PolynomialRing(GF(32003),order=TermOrder(Matrix([1,2,0,3])))
sage: P.term_order()
m(1,2,0,3) term order This uses the non-standard "m(...)" representation which I would avoid. I'd be happy with either "M()" (Singular notation) or "Matrix term ordering with matrix ..." or so. Also, the "m()" notation is allowed but shouldn't. sage: P.<a,b> = PolynomialRing(GF(32003),order='m(1,2,0,3)') I understand that this is a typical paint-the-bike-shed scenario and in particular a question of choice. Still, I think we shouldn't invent more ad-hoc string notation when (a) there is an established notation and (b) we have a much nicer object oriented way of constructing term orderings. However, since this isn't really that big of a deal, I am okay with being overruled by some other referee who disagrees. PS: Apologies for taking so long to revisit this ticket. |
comment:18
Replying to @malb: I prefer "Matrix term ordering with matrix ...". both "M(1,2,0,3)" and "m(1,2,0,3)" is allowed, which is not bad. The situation is the same with other orderings like "Lex", which is converted to lower case internally. I don't mind that "M(...)" should be the official string representation of matrix orderings. I am working to make matrix ordering work with Singular version. I am not really confident whether the code is sound as it is based on a knowledge obtained by a reverse engineering of libSingular Sage interface. I will upload a revised patch soon, perhaps within a couple of hours. Then I wish you again to review the patch. |
with singular version support |
comment:19
Attachment: trac#6922.2.patch.gz The new patch supports also the Singular version. Now "M(...)" is the official string representation of matrix orderings. |
comment:20
Replying to @kwankyu:
Good point, I'm convinced.
Great, I'll take a look. |
comment:21
Patch looks good (small issue see below), applies cleanly, doctests pass. So, this stuff now works, very cool: sage: P = PolynomialRing(GF(127),2,'x',order=Matrix([1,2,0,3]))
sage: P.term_order()
Matrix term order with matrix
[1 2]
[0 3]
sage: magma(P)
Polynomial ring of rank 2 over GF(127)
Order: Weight [full]
Variables: x0, x1
sage: singular(P)
// characteristic : 127
// number of vars : 2
// block 1 : ordering M
// : names x0 x1
// : weights 1 2
// : weights 0 3
// block 2 : ordering C I've attached a small referee patch which Kwankyu or someone else has to sign of. Kwankyu's patch gets a positive review modulo the issue in the referee patch. |
Attachment: trac_6922_referee.patch.gz apply after klee's patch |
Attachment: trac#6922_final.patch.gz combined with referee's patch |
comment:22
Positive review for the referee's patch. Thank you! |
Reviewer: Martin Albrecht |
comment:24
I'm applying just |
Merged: sage-4.5.2.alpha0 |
In Sage, I am trying to construct a polynomial ring with matrix
ordering.
....
AFAIK, it is not implemented, but I think that some people were
working on it.
It is in fact one of the things that I miss in Sage's polynomial rings
(the other thing are supercommutative rings), so that I need to use
the Singular interface rather than libsingular.
....
Anyway it will be great that the matrix ordering is included in Sage
natively.
CC: @novoselt
Component: basic arithmetic
Keywords: term order
Author: Kwankyu Lee
Reviewer: Martin Albrecht
Merged: sage-4.5.2.alpha0
Issue created by migration from https://trac.sagemath.org/ticket/6922
The text was updated successfully, but these errors were encountered: