Add minimalPrimes strategy for ideals in ZZ and ZZ/n#4249
Merged
d-torrance merged 5 commits intoMacaulay2:developmentfrom May 7, 2026
Merged
Add minimalPrimes strategy for ideals in ZZ and ZZ/n#4249d-torrance merged 5 commits intoMacaulay2:developmentfrom
d-torrance merged 5 commits intoMacaulay2:developmentfrom
Conversation
mahrud
reviewed
May 3, 2026
|
|
||
| presentation PolynomialRing := Matrix => R -> map(R^1, R^0, 0) | ||
| presentation Ring := | ||
| presentation RingFamily := Matrix => presentation @@ module |
Member
There was a problem hiding this comment.
@@ is unnecessarily opaque, as it makes it hard to see the code.
Also, I'm worried about someone defining some other type of non-polynomial ring, forgetting to define presentation for it, and because of this line always getting that the presentation of their ring is trivial.
Member
There was a problem hiding this comment.
Yeah, this seems to be a problem with at least SheafOfRings:
i40 : (showStructure())#Thing#HashTable#MutableHashTable#Type#Ring
o40 = EngineRing : FractionField
FreeAlgebra
GaloisField
InexactField : ComplexField
RealField
LocalRing
PolynomialRing
QuotientRing : FreeAlgebraQuotient
SchurRing
NCRing : NCPolynomialRing
NCQuotientRing
SheafOfRings
Member
Author
There was a problem hiding this comment.
Good point. I've updated it so that (presentation, Ring) first looks for the presence of a presentation key in the ring object (which is only installed for ZZ and QQ and w/o using @@), runs it if it's there, and raises an error otherwise:
i1 : presentation ZZ
o1 = 0
1
o1 : Matrix ZZ <-- 0
i2 : presentation QQ
o2 = 0
1
o2 : Matrix QQ <-- 0
i3 : presentation new Ring
stdio:3:12:(3):[1]: error: presentation for this ring not implemented yetGeneralize it for all rings (not just polynomial rings) and ring families, so "presentation ZZ" and "presentation RR" work. Also call (presentation, Module) so caching happens out of the box and update (presentation, QuotientRing) so that it caches in the cache table like the other presentation methods.
Also drop Constant ^ RingFamily; redundant since Constant is a subclass of Number.
Member
Author
|
Discussed with Anton and Mike |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This closes #3505:
We also add a few related updates I encountered along the way:
module(RingFamily)somodule RRandmodule CCwork.presentation(Ring)(andpresentation(RingFamily)sopresentation ZZworks. It callspresentation(Module), so the result gets cached.RingElement^Ringas syntactic sugar forlift(matchingNumber^Ring)ZZ.