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

Fixed #3600 #3611

Merged
merged 2 commits into from
Dec 27, 2024
Merged

Fixed #3600 #3611

merged 2 commits into from
Dec 27, 2024

Conversation

felixlotter
Copy link

The problem was that after loading NCAlgebra, putting a list next to a ring creates a non-commutative polynomial ring. Adding "new Array from" fixed the problem.

@d-torrance
Copy link
Member

Does this also fix #3601? It would probably also be good to add a test that shows that the bug is fixed.

@felixlotter
Copy link
Author

felixlotter commented Dec 16, 2024

Does this also fix #3601?

The problem in #3601 is independent of NCAlgebra, so unfortunately this is not fixed.

EDIT: I found the problem and solved it by using a dictionary. But I don't know if this is the best/standard way to do this?
EDIT: Unfortunately, the fix created new problems. I reverted the commits for now.

@d-torrance
Copy link
Member

@mikestillman - Does this change look okay to you?

Comment on lines 754 to 762
R1Gens := gens R1;
numDigits := length (toString (#R1Gens));
varstr := (for i in 1..#R1Gens list "x" | demark ("",for j from 1 to numDigits-(length toString i) list "0") | toString i);
var := new Dictionary;
varlist = for i in varstr list getGlobalSymbol(var,i);
R2 := (coefficientRing R1) new Array from varlist;
R2Gens := gens R2;
generatorMapping := for i in 0..#(gens R1) - 1 list (R1Gens#i =>R2Gens#i);
return (map(R2, R1, generatorMapping), R2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's something wrong with the indentation here, which makes it look like you edited every line here. Could you revise this commit? (I can show you tomorrow if you don't know how to do this)

@d-torrance
Copy link
Member

The StatePolytope tests are failing now:

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)

This method calls gfan from gfanInterface:

o3 = -- code for method: initialIdeals(Ideal)
     /usr/share/Macaulay2/StatePolytope.m2:35:24-35:51: --source code:
     initialIdeals(Ideal) := (I) -> apply(gfan I, first)

@d-torrance
Copy link
Member

The new test is failing:

 -- -- -*- M2-comint -*- hash: 741132826
 -- 
 -- i1 : 
 --      	R = QQ[a];
 -- 
 -- i2 : 	gfanConvertToNewRing(R);
 -- stdio:3:28:(3): error: no method for adjacent objects:
 --             gfanConvertToNewRing (of class Symbol)
 --     SPACE   R (of class PolynomialRing)
 -- 
stdio:1:5:(3): error: test(s) #2 of package gfanInterface failed.

Either gfanConvertToNewRing could be exported or it could be loaded in the test:

importFrom(gfanInterface, "gfanConvertToNewRing")

Update M2/Macaulay2/packages/gfanInterface.m2

fixed test

Co-authored-by: Mahrud Sayrafi <[email protected]>

fixed test

added test
Copy link
Member

@mikestillman mikestillman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks!

@d-torrance d-torrance merged commit 27cf58a into Macaulay2:development Dec 27, 2024
5 checks passed
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

Successfully merging this pull request may close these issues.

5 participants