Opened 6 years ago
Closed 4 years ago
#22809 closed enhancement (fixed)
Pass number of variables to polygens
Reported by:  mforets  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  commutative algebra  Keywords:  polynomial vector 
Cc:  nbruin, tscrim, vdelecroix  Merged in:  
Authors:  Marcelo Forets  Reviewers:  Simon King 
Report Upstream:  N/A  Work issues:  
Branch:  5f1a4da (Commits, GitHub, GitLab)  Commit:  5f1a4da2392e198878f33f3bc8fc118178dd6218 
Dependencies:  Stopgaps: 
Description
The number of generators of a polynomial can be specified as a number:
sage: PolynomialRing(QQ, 'x', 4).gens() (x0, x1, x2, x3)
This ticket should allow doing:
sage: polygens(QQ, 'x', 4) (x0, x1, x2, x3)
Currently it returns TypeError: polygens() takes at most 2 arguments (3 given)
.
Change History (27)
comment:1 Changed 6 years ago by
Branch:  → u/mforets/pass_number_of_variables_to_polygens 

comment:2 Changed 6 years ago by
Commit:  → df2e39f6096e63e18a7014717e44a2013450cb36 

comment:3 followup: 5 Changed 6 years ago by
Hmm, I wasn't aware of this other functionality. I have no opinion about it per se, but if you do it, I wonder if you could experiment with what might happen if one did
L = polygens(QQ, 'x', 4) for l in L: SR(l).inject_variable()
or whatever the right commands would be for that. Or even
polygens(SR, 'x', 4)
though I assume that wouldn't work right. See #11576 for motivation. (Or forget this comment and just be aware of #11576, which had a flurry of activity four years ago but hasn't been looked at since.)
comment:4 Changed 6 years ago by
Commit:  df2e39f6096e63e18a7014717e44a2013450cb36 → 6d2082c66faeadc4d569afbc02a5b2fb52bc741b 

Branch pushed to git repo; I updated commit sha1. New commits:
6d2082c  allow inject_variables and start_at options

comment:5 Changed 6 years ago by
Replying to kcrisman:
Hmm, I wasn't aware of this other functionality. I have no opinion about it per se, but if you do it, I wonder if you could experiment with what might happen if one did
L = polygens(QQ, 'x', 4) for l in L: SR(l).inject_variable()or whatever the right commands would be for that. Or even
polygens(SR, 'x', 4)though I assume that wouldn't work right.
great idea! it is very useful that these variables 'exist' in the calling method.
as you can see in revised patch i used get_main_globals
to get around the issue.
how does it look? does someone suggest other examples?
See #11576 for motivation. (Or forget this comment and just be aware of #11576, which had a flurry of activity four years ago but hasn't been looked at since.)
ok, thanks for pointing that out! although it seems to me that #11576 is an advanced version of the simpler (but yet lacking) uses var('x', 4)
and var('x', 4, 4)
for vectors and matrices respectively. or i'm missing some basic reason why they don't exist already?
comment:6 Changed 6 years ago by
Don't both return a result and inject bindings (and certainly don't go grabbing the global scope!). It's pretty clear that the behaviour of var
to both inject and return variables is a mistake once you see the confusion it causes.
Injection of polynomial variables is quite efficiently done with PolynomialRing(R,'x',4).inject_variables()
, which prints a message and returns None
on purpose.
polygen(*args)
is just a short form of PolynomialRing(*args).gens()
which helps people who know what a polynomial variable is but not what a polynomial ring is. I think it's a bad design decision to muddle this with other features.
I'm also against providing options to modify the numbering. There's a good reason why RX=PolynomialRing(R,'x',n)
is allowed: it's clearly desirable to make "a polynomial ring in n variables" in various settings. Sage requires names for these variables, but specifying n names is bothersome if you don't care about the names. Hence the indexing. which agrees with what RX.gen(i)
does. If you do care what your variable names are called, it's easy to specify a list of names. All kinds of possibilities exist: ["x_%s"%s for i in range(n)]
, ["x^(%s)"%s for i in range(1,n+1)]
etc. It's much better to teach people how to get full customizibility than to give them one more option to discover, which will probably not meet their needs if they develop further.
comment:7 followup: 8 Changed 6 years ago by
@nbruin : thanks for the prompt feedback :) but are you ok with df2e39f or not? notice that *args
was missing there on the first place.
Don't both return a result and inject bindings (and certainly don't go grabbing the global scope!).
is there an alternative to global_scope()
? i tried with just R.inject_variables()
and it didn't work. i suppose it works via return R.injected_variables()
, but then you don't have access to your vector of variables, right? for multiplication and so on, it is useful to have it.
It's pretty clear that the behaviour of
var
to both inject and return variables is a mistake once you see the confusion it causes.
could you point me to some threads on this issue, so that i can catch up? for me there are no surprises with var
. moreover, i find it practical that the function returns you the variables and/or allows you to use them in the console.
Injection of polynomial variables is quite efficiently done with
PolynomialRing(R,'x',4).inject_variables()
, which prints a message and returnsNone
on purpose.
ok, but let's agree it is a rather long construction for people with bad memory (i'm in that group).
polygen(*args)
is just a short form ofPolynomialRing(*args).gens()
which helps people who know what a polynomial variable is but not what a polynomial ring is. I think it's a bad design decision to muddle this with other features.
yes (essentially i'm on that group, too). that's arguable: why should a feature bother at all? so long as it is consistent with the rest & well documented.. (more on this below)
If you do care what your variable names are called, it's easy to specify a list of names. All kinds of possibilities exist:
["x_%s"%s for i in range(n)]
,["x^(%s)"%s for i in range(1,n+1)]
etc.
that's definitely not easy.
It's much better to teach people how to get full customizibility than to give them one more option to discover, which will probably not meet their needs if they develop further.
i get the point about "teaching a man to fish", 100%. at the same time, i'm with the lazy people here: if someone thought the solution and shows you a little example in command help, then i prefer using it rather than working it out myself (saves time, and is less errorprone).
for the point on features: what about LaTeX? in general people are happy with lots and lots of customizations ready to be discovered.
comment:8 followup: 10 Changed 6 years ago by
Replying to mforets:
@nbruin : thanks for the prompt feedback :) but are you ok with df2e39f or not? notice that
*args
was missing there on the first place.
Yes, that looks pretty straightforward. It might be that passing *args
on wholesale is problematic (it causes erroneous parameters to be reported with a deeper traceback), but I don't see a big problem immediately. That's also something that is easy to fix later.
Concerning your other queries:
 in python it is usual that routines that are called because of their sideeffects don't return anything. Compare
L.sort()
versussorted(L)
 just google or search the documentation for the number of examples of
y=var('y')
etc.  I don't think you will find many people who would give LaTeX as an example of a welldesigned interface.
 implementing extra features adds bloat and other maintenance burden, and makes it more confusing for new people, because now they will be confronted with multiple ways of doing things. See "The zen of python". Not all of it applies to math software, but generally the ideas there have worked well to keep python a pleasant and easy to learn language.
That said, nothing prevents you from making your own convenience library functions and maintaining them in your own installation. Once they have been sitting around for a year or two and you're finding that you use them often, you'll have a nice body of code to demonstrate in exactly what way they are more convenient. Practice learns that what seems convenient when you're writing/designing something is often not used much in actual use.
Sage is getting large and mature enough that if there is a reasonable way of doing something, we should be hesitant about adding alternative constructions.
comment:9 Changed 6 years ago by
ok, thanks for pointing that out! although it seems to me that #11576 is an advanced version of the simpler (but yet lacking) uses var('x', 4) and var('x', 4, 4) for vectors and matrices respectively. or i'm missing some basic reason why they don't exist already?
Because nobody has both implemented and reviewed them. Lots of bikeshedding around exactly what the defaults and syntax should be.
comment:10 followup: 11 Changed 6 years ago by
Replying to nbruin:
Replying to mforets:
@nbruin : thanks for the prompt feedback :) but are you ok with df2e39f or not? notice that
*args
was missing there on the first place.Yes, that looks pretty straightforward. It might be that passing
*args
on wholesale is problematic (it causes erroneous parameters to be reported with a deeper traceback), but I don't see a big problem immediately. That's also something that is easy to fix later.
got it. df2e39f does solve the ticket's requirement. yet i suggest to leave it in the current status for 1 week in case someone else want to chime in.
BTW I'm willing to learn and apply the more robust solution if you have time now (or in the future).
That said, nothing prevents you from making your own convenience library functions and maintaining them in your own installation. Once they have been sitting around for a year or two and you're finding that you use them often, you'll have a nice body of code to demonstrate in exactly what way they are more convenient. Practice learns that what seems convenient when you're writing/designing something is often not used much in actual use.
Yes, I need a year or two (more than just time, certainly!) to just grasp what is convenient or not in terms of good design from an expert Python / Sage developer's standpoint.
No, I don't need a year or two to know that any of the possibilities that you suggest ["x_%s"%s for i in range(n)]
, ["x^(%s)"%s for i in range(1,n+1)]
are not easy. Just because of this:
 Maple (born 1982) 
Vector(n, symbol = x)
 Matlab (born 1984) 
sym('x', [n 1])
 Mathematica (born 1988) 
Array[x, n]
 SymPy? (born 2007) 
symbols('x:n')
.
Matrices are constructed in a similar way.
comment:11 followup: 12 Changed 6 years ago by
Replying to mforets:
 Maple (born 1982) 
Vector(n, symbol = x)
 Matlab (born 1984) 
sym('x', [n 1])
 Mathematica (born 1988) 
Array[x, n]
 SymPy? (born 2007) 
symbols('x:n')
.Matrices are constructed in a similar way.
Doesn't vector(polygen(QQ,'x',n))
and matrix(n,m,polygen(QQ,'x',n*m))
fit the bill for that? We need a little more symbols, but that's because sage is more explicit about the ring over which one works.
comment:12 followup: 13 Changed 6 years ago by
Replying to nbruin:
Replying to mforets:
 Maple (born 1982) 
Vector(n, symbol = x)
 Matlab (born 1984) 
sym('x', [n 1])
 Mathematica (born 1988) 
Array[x, n]
 SymPy? (born 2007) 
symbols('x:n')
.Matrices are constructed in a similar way.
Doesn't
vector(polygen(QQ,'x',n))
andmatrix(n,m,polygen(QQ,'x',n*m))
fit the bill for that? We need a little more symbols, but that's because sage is more explicit about the ring over which one works.
Oh yes, that's a nice construction for \mathcal{M}_{n\times m}(\mathbb{K}[x])
(minor: it is the plural, polygens
).
Another thing that the commercial software have in common is that they follow mainstream Linear Algebra convention about variable names. That construction doesn't, especially:
sage: matrix(2, 4, polygens(QQ, 'x', 8)) [x0 x1 x2 x3] [x4 x5 x6 x7]
What do you people think about the following. There is an open ticket #13703 which targets common matrix constructors. In the same spirit as the polytope library (type polytopes.?
), with some work we can have a comprehensive matrix library, matrices.?
, that features in particular:
sage: matrices.symbolic(2) [a11 a12] [a21 a22]
comment:13 followup: 14 Changed 6 years ago by
Replying to mforets:
What do you people think about the following. There is an open ticket #13703 which targets common matrix constructors. In the same spirit as the polytope library (type
polytopes.?
), with some work we can have a comprehensive matrix library,matrices.?
, that features in particular:sage: matrices.symbolic(2) [a11 a12] [a21 a22]
What would it be used for? If the matrix is of any significant size, there is not much computation you can do with these things. And if the matrix is small you might as well construct it manually. In any case:
 you'd want to be able to specify what the base ring is (or specify you want elements of SR as entries)
 once you're making these "universal" matrices, you should probably be able to specify the form (symmetric, antisymmetric, upper triangular, etc.)
 The labelling should be 0based. You'd want
M[1,2]
to bea12
.
comment:14 Changed 6 years ago by
Replying to nbruin:
Replying to mforets:
What do you people think about the following. There is an open ticket #13703 which targets common matrix constructors. In the same spirit as the polytope library (type
polytopes.?
), with some work we can have a comprehensive matrix library,matrices.?
, that features in particular:sage: matrices.symbolic(2) [a11 a12] [a21 a22]What would it be used for?
 Teaching : undergraduate level or before using SageMath.
 Mathematical Modeling : to represent an optimization variable in a semidefinite program.
I'd be delighted to go deeper into any of these use cases, or to further exchange our ideas and ideals about less concrete, openended issues that innocent #22809 has raised, such as why this very basic construct exists in all the software cited before, while we shouldn't expect a useful answer for the inverse of a big matrix whose coefficients are variables.
But, is this relevant at all to #22809? Maybe in Sagedevel?
comment:15 Changed 6 years ago by
Commit:  6d2082c66faeadc4d569afbc02a5b2fb52bc741b → 7a09c12b7f8b1ff03fdf2093d1f57d41bd622974 

Branch pushed to git repo; I updated commit sha1. New commits:
7a09c12  Revert "allow inject_variables and start_at options"

comment:16 followup: 18 Changed 6 years ago by
fair enough about naming conventions or handy constructions. as far as i'm concerned, this ticket is ready for peerreview!
side note: i've used git revert
as suggested here trying to avoid messing up history (*). please let me know if this is not the correct way in this context.
(*) for the record: checked chapter 1 of the Guide, but didn't find this use case.
comment:17 Changed 6 years ago by
Status:  new → needs_review 

ps: sorry, next time i'll write the updates message in this place so that you don't receive 2 emails..
comment:18 followup: 19 Changed 6 years ago by
Replying to mforets:
side note: i've used
git revert
as suggested here trying to avoid messing up history (*). please let me know if this is not the correct way in this context.
You can do that if you care about preserving in the history that a change was made and rolled back. In a case like this I would just go with the branch that does NOT have the changes that were rolled back (rebase, or otherwise rebuild your branch into the state you want directly). Rebasing becomes a real problem if other people have branches based on your branch. I don't think that's an issue here.
comment:19 followup: 20 Changed 6 years ago by
Replying to nbruin:
Replying to mforets:
side note: i've used
git revert
as suggested here trying to avoid messing up history (*). please let me know if this is not the correct way in this context.You can do that if you care about preserving in the history that a change was made and rolled back. In a case like this I would just go with the branch that does NOT have the changes that were rolled back (rebase, or otherwise rebuild your branch into the state you want directly). Rebasing becomes a real problem if other people have branches based on your branch. I don't think that's an issue here.
ok. may i ask for some extra help here please, like an example? the one in page 19 of the Guide doesn't seem to treat this case. (i did some wrong stuff with git in days84, so i pretend to be more cautious!.)
comment:20 Changed 6 years ago by
Replying to mforets:
Replying to nbruin:
Replying to mforets:
side note: i've used
git revert
as suggested here trying to avoid messing up history (*). please let me know if this is not the correct way in this context.You can do that if you care about preserving in the history that a change was made and rolled back. In a case like this I would just go with the branch that does NOT have the changes that were rolled back (rebase, or otherwise rebuild your branch into the state you want directly). Rebasing becomes a real problem if other people have branches based on your branch. I don't think that's an issue here.
ok. may i ask for some extra help here please, like an example? the one in page 19 of the Guide doesn't seem to treat this case. (i did some wrong stuff with git in days84, so i pretend to be more cautious!.)
In this case it would seem to be that a hard reset (as described on the same page) and a forced push to the branch on this ticket would be fine. Anyone who has work based on your branch will be seriously inconvenienced, but I expect there to be noone in that group here. Note that your actual change is (2/+7) lines and that with the revert, you'd be archiving the adding and removal of an additional 42 lines.
As is described in many places, as soon as you publish a git branch you should be careful rewriting its history, because you could affect people quite badly. However, in this case, with changes so small and localized, any rebase merge would be really simple (and probably handled automatically), so I'd opt for publishing a cleaner history. People shouldn't really be basing work off branches for tickets that are still developing, unless there's a good reason for it.
comment:21 Changed 6 years ago by
Commit:  7a09c12b7f8b1ff03fdf2093d1f57d41bd622974 → 5f1a4da2392e198878f33f3bc8fc118178dd6218 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5f1a4da  add optional args to polygens

comment:22 Changed 6 years ago by
Authors:  → Marcelo Forets 

comment:23 Changed 6 years ago by
Cc:  tscrim vdelecroix added 

@nbruin : thanks for your detailed answer concerning git reset. i've done what you suggested.
All: do you have any further inputs with respect to this ticket? thanks.
comment:24 Changed 6 years ago by
Component:  calculus → commutative algebra 

comment:25 Changed 6 years ago by
friendly reminder for ticket review  seemingly green light from Nils five weeks ago.
thanks
comment:26 Changed 4 years ago by
Reviewers:  → Simon King 

Status:  needs_review → positive_review 
Although I have never used polygens
, I'd say it is an obviously reasonable idea to allow the same optional arguments that are available for the polynomial ring constructor. It doesn't break anything, the patchbot is happy, and it seems to me that 2 years ago people just forgot to set it to positive review. Doing it now. If Nils want to have his name as a reviewer added, please go ahead.
comment:27 Changed 4 years ago by
Branch:  u/mforets/pass_number_of_variables_to_polygens → 5f1a4da2392e198878f33f3bc8fc118178dd6218 

Resolution:  → fixed 
Status:  positive_review → closed 
apart from that, i'm tempted to propose making this possible too:
New commits:
add optional argrs to polygens