Macaulay2 / M2

The primary source code repository for Macaulay2, a system for computing in commutative algebra, algebraic geometry and related fields.
https://macaulay2.com
343 stars 230 forks source link

bad behavior (bug): substitute using a matrix of values #113

Open mikestillman opened 10 years ago

mikestillman commented 10 years ago

substituting using a matrix (not Options, not a ring map) can still give the wrong/unexpected answer. Here are two situations (one over QQ, the other over CC_53).

It should be easy to fix this, Dan, as the "Options" version was changed to produce these correct values recently.

R = QQ[x]
m = matrix{{1/2 * x}}
sub(m, {x=>1_ZZ}) -- is correct behavior
sub(m, matrix{{1}}) -- just wrong.  Should give behavior on previous line
f = map(ZZ, R, {1}) -- REALLY: should give an error: cannot construct ring map from R --> ZZ.
f m -- wrong behavior currently.  But if f cannot be constructed, this will not be a problem
g = map(QQ[y], R, {1}) -- this one works now, and is the behavior we want
g m

R = CC[x]
m = matrix{{ii * x}}
sub(m, {x=>1_ZZ}) -- is correct behavior
sub(m, matrix{{1}}) -- just wrong.  Should give behavior on previous line
f = map(RR_53, R, {1}) -- REALLY: should give an error.
f m -- crashes in linalg branch, gives 0 in 1.6. Both are wrong...!
g = map(CC_53[y], R, {1}) -- this one works now, and is the behavior we want
g m
DanGrayson commented 10 years ago

I forget why we didn't fix the engine so it doesn't do substitutions that are not ring maps.

mikestillman commented 10 years ago

The engine ring maps are not full ring maps: they only know their target ring, and where to send the i-th variable.

For better or worse, the reason we allow non ring maps is, if I remember correctly, to allow lifting of matrices over ZZ/2, or ZZ/p, to matrices over some other ring. This is a poor reason, most likely.

Here, I suggest we do the following:

  1. the engine should (check and) refuse to do the evaluation at least in the cases: QQ --> ZZ, CC --> RR (and polynomial rings based on these).
  2. the front end should create the correct ring map in the case when a matrix of values is given.

Does our 'lift' function work from ZZ/2[x,y,z] lifting to QQ[x,y,z]? If so, maybe we could disallow non-mathematical ring maps. But this will likely break people's code.

DanGrayson commented 10 years ago

No, lift won't do that case. Or even lift from ZZ/2[x] to ZZ[x]. But it should lift from ZZ[x]/2 to ZZ[x].

On Wed, Mar 5, 2014 at 3:30 PM, Mike Stillman notifications@github.comwrote:

The engine ring maps are not full ring maps: they only know their target ring, and where to send the i-th variable.

For better or worse, the reason we allow non ring maps is, if I remember correctly, to allow lifting of matrices over ZZ/2, or ZZ/p, to matrices over some other ring. This is a poor reason, most likely.

Here, I suggest we do the following:

  1. the engine should (check and) refuse to do the evaluation at least in the cases: QQ --> ZZ, CC --> RR (and polynomial rings based on these).
  2. the front end should create the correct ring map in the case when a matrix of values is given.

Does our 'lift' function work from ZZ/2[x,y,z] lifting to QQ[x,y,z]? If so, maybe we could disallow non-mathematical ring maps. But this will likely break people's code.

— Reply to this email directly or view it on GitHubhttps://github.com/Macaulay2/M2/issues/113#issuecomment-36789711 .

mikestillman commented 8 years ago

Dan, is it possible to disallow the creation of the 'f' maps above (first message in thread)? There is still a crash in here. I should change the engine so it doesn't crash. I could also give an error during evaluation of a ring map in the engine so that it refuses to do that, in the case when the evaluation has base CC --> RR, QQ --> ZZ, RR --> ZZ, or CC-->ZZ.

DanGrayson commented 8 years ago

That seems pretty ad hoc. This sounds like an unimportant problem that we can postpone to 2.0, so we can do something better.