Skip to content
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

gfanInterface: Another issue with gfanConvertToNewRing #3601

Open
felixlotter opened this issue Nov 28, 2024 · 4 comments
Open

gfanInterface: Another issue with gfanConvertToNewRing #3601

felixlotter opened this issue Nov 28, 2024 · 4 comments

Comments

@felixlotter
Copy link

felixlotter commented Nov 28, 2024

While fighting with #3600 I discovered a more exotic bug, unrelated to NCAlgebra, but probably coming from the same method. To produce a minimal example I copied some code from MultigradedImplicitization.m2 (@bkholler):

needsPackage "gfanInterface"

maxGrading = method(Options => {ReturnTargetGrading => false});
maxGrading RingMap := Matrix => opts -> F -> (
    dom := source F;
    codom := target F;
    elimRing := dom ** codom;
    X := vars dom;
    n := numgens dom;
    elimIdeal := ideal(sub(X, elimRing) - sub(F(X), elimRing));
    linealitySpace(gfanHomogeneitySpace(elimIdeal))
)

R1 = QQ[x1]
R2 = QQ[s1,s2]
maxGrading(map(R1,R2,{x1,x1^2})) -- works fine once
maxGrading(map(R1,R2,{x1,x1^2})) -- error

Here, the error message is

error: no method found for applying promote to:
     argument 1 :  x1 (of class QQ {x1, x2, x3})
     argument 2 :  R1

The problem seems to be the variable name "x1", which is also used inside gfanConvertToNewRing. Interestingly, calling gfanHomogeneitySpace directly, e.g. does not produce any errors. It does, too.

felixlotter pushed a commit to felixlotter/M2 that referenced this issue Dec 16, 2024
felixlotter pushed a commit to felixlotter/M2 that referenced this issue Dec 16, 2024
@felixlotter
Copy link
Author

The problem is that when gfanConvertToNewRing creates the new ring, it calls "use" on this ring, which changes the ring of x1 globally.
Is there any way to make creating new rings and working with rings inside a package safe? Ideally, some local form of "use" that only installs ring variables and operations inside the scope of a package or method?

@d-torrance
Copy link
Member

Calling monoid should do the trick, i.e., R monoid M instead of R M. The latter calls use on the former:

i1 : code (symbol SPACE, Ring, Array)

o1 = -- code for method: Ring Array
     src/macaulay2/M2/M2/Macaulay2/m2/polyrings.m2:138:33-138:57: --source code:
     Ring Array  := PolynomialRing => (R, M) -> use R monoid M

@felixlotter
Copy link
Author

The problem with this solution is that the symbols in M are not interpreted as variables afterwards; so no ring operations are available. If I am not mistaken this is exactly where the error message in the failed test from #3611 came from.

i1 : R = QQ[a,b];
i2 : I = ideal(a^2+b^2,a*b);
o2 : Ideal of R
i3 : initialIdeals(I)  
currentString:1:2:(3):[9]: error: no method for binary operator * applied to objects:
            x1 (of class Symbol)
      *     x2 (of class Symbol)

I guess one could rewrite gfanInterface.m2 to only access the ring variables as R_i... but a local version of use would be much more convenient (and in general a desirable feature I think!). I imagine something like this:

  • useInScope installs ring variables and operations to the scope of the method.
  • Only if the method returns something that references the local ring variables (e.g. a polynomial), the variables of the local scope are merged into the scope from which the method was called, changing the rings of existing variable symbols if necessary.

While this returns false:

foo = method();
foo(ZZ) := z-> (R := QQ[x,y]; f := x*y; return("This should act like a blackbox."));

R = QQ[x];
foo(1);
ring x === R

This should return true:

foo = method();
foo(ZZ) := z-> (R := QQ(monoid [x,y]); useInScope(R); f := x*y; return("This should act like a blackbox."));

R = QQ[x];
foo(1);
ring x === R

I don't know if something like this could be implemented?

@felixlotter
Copy link
Author

felixlotter commented Dec 18, 2024

I think I found the solution! One needs to use the keyword local, then the usual use will act as the "local form of use" that I was looking for. The following returns true:

foo = method();
foo(ZZ) := z-> (R := QQ[local x, local y]; f := x*y; return("This should act like a blackbox."));

R = QQ[x];
foo(1);
ring x === R

EDIT: Actually, not quite. I would also want that

foo = method();
foo(ZZ) := z-> (R := QQ[local x, local y]; return(R));

R = QQ[x];
use foo(1);
ring x === R

returns false... but it doesn't.

felixlotter pushed a commit to felixlotter/M2 that referenced this issue Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants