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.

Sign up for GitHub

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

Jump to bottom

Injective and irreducible resolutions of Q-graded modules #4100

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Injective and irreducible resolutions of Q-graded modules #4100

wants to merge 5 commits into from
+1,006 −0

Conversation

tom111
Copy link

@tom111 tom111 commented Sep 16, 2024

This is a draft pull request as recommended here.

Anna Hofer @annahofer00 has been working on an implementation of the algorithms of Helm/Miller and here we are now trying to put this into OSCAR. At the moment this code can compute some irreducible resolutions, but polyhedral pre-computation is done by hand. This needs to be automated.

The code also relies on the computation of certain colon-modules that is not yet available in OSCAR but maybe it should be.

Specifically, if I is an ideal in a ring R and M an R-Module then we need 0 :_M I = {m \in M : mI = 0}. As a module this is isomorphic to Hom(R/I, M) but one has to be careful with degree information (in the graded case). I think it would be good to implement a general purpose "module quotient" function like this one in Macaualy2, which has the very practical operator : for this purpose. Many of these colons also have a saturation, where this is repeated to stabilization.

Any comments are welcome of course. We'll keep adding to this branch for now.

@joschmitt joschmitt marked this pull request as draft September 16, 2024 20:12
@joschmitt joschmitt changed the title Draft pull request for injective and irreducible resolutions of Q-graded modules Injective and irreducible resolutions of Q-graded modules Sep 16, 2024
@joschmitt joschmitt added the experimental Only changes experimental parts of the code label Sep 16, 2024
fingolfin
fingolfin reviewed Sep 16, 2024
View reviewed changes
experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated
Comment on lines 100 to 101
A = Nothing
b = Nothing
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
A = Nothing
b = Nothing

experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated

Given a homogeneous prime ideal, return the corresponding face F

INPUT: prime ideal p_F
Copy link
Member

Choose a reason for hiding this comment

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

prime ideal in what kind of ring?

experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated
#get generators of k[F] = k[Q]/P_F
Q_F,_ = quo(R_Q,P_F)
G_F = gens(Q_F)
L_F = (P_F,[])
Copy link
Member

@fingolfin fingolfin Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I am confused. You never access L_F[1]. Could you perhaps do this

Suggested change
L_F = (P_F,[])
L_F = []

and then below replace L_F[2] by just L_F?

In addition [] is a Vector{Any} which isn't great for type stability.

experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated
function get_all_ass_primes(I)
R_Q = base_ring(I)
I_m = ideal(R_Q,gens(R_Q)) #maximal ideal
F_m = get_face_to_prime(I_m)[1]
Copy link
Member

Choose a reason for hiding this comment

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

Why does get_all_ass_primes return multiple values if you only use the first? Perhaps it should also return a FaceQ as its name suggests?

experimental/InjectiveResolutions/src/InjectiveResolutions.jl
# some composite types
#########################

struct FaceQ # face of semigroup
Copy link
Member

Choose a reason for hiding this comment

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

What is a "face of semigroup" ? And what does the Q stand for here?

experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated
else
R_poly = base_ring(R_N)
end
T = typeof(zero(R_N))
Copy link
Member

Choose a reason for hiding this comment

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

This really should be

Suggested change
T = typeof(zero(R_N))
T = elem_type(R_N)

also elsewhere in similar situations

experimental/InjectiveResolutions/src/InjectiveResolutions.jl
Comment on lines +388 to +390
if is_zero(J)
return I
else
Copy link
Member

Choose a reason for hiding this comment

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

More idiomatic:

Suggested change
if is_zero(J)
return I
else
is_zero(J) && return I

experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated
Comment on lines 392 to 395
G = []
for g in gens(I)
push!(G,monomial_basis(S,degree(g))[1])
end
Copy link
Member

Choose a reason for hiding this comment

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

More idiomatic and also more type stable (i.e., the vector this produces not always a Vector{Any}) is to use a list comprehension:

Suggested change
G = []
for g in gens(I)
push!(G,monomial_basis(S,degree(g))[1])
end
G = [monomial_basis(S,degree(g))[1] for g in gens(I)]

experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated
Comment on lines 409 to 412
V = Vector{T}()
for g in gens(I)
push!(V,g*f)
end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
V = Vector{T}()
for g in gens(I)
push!(V,g*f)
end
V = [g*f for g in gens(I)]

experimental/InjectiveResolutions/src/InjectiveResolutions.jl
#alternative to quotient_ring_as_module(I), which gives wrong result when base_ring(I) = quotient ring
#INPUT: ideal I
#OUTPUT: quotient S/I as SubquoModule (S = base_ring(I))
function quotient_by_ideal(I::Ideal)
Copy link
Member

Choose a reason for hiding this comment

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

The name of this function and the comment leave me really confused. This is not something that should be reachable as Oscar.quotient_by_ideal. Might be acceptable if hidden in a submodule.

@fingolfin
Copy link
Member

Last night I was a bit terse in what I wrote here, so I'd like to clarify that overall I think this looks like a very valuable pull request, thank you for starting on it! My comments are all on a relatively low-level technical level, and I think most of them are trivial to address, but also not urgent, i.e. it is fine to only address them gradually. Also feel free to ask resp. push back if something is unclear or seems unreasonable to you.

As an example: the name FaceQ of course is fine if you use it internally and know what you do, but long term others may have to interact with this code, and then either a more descriptive name, or else a comment that explains the name, can go a long way.

Anyway, thanks again for making this PR in the first place and I look forward to seeing how it evolves.

fingolfin
fingolfin reviewed Sep 17, 2024
View reviewed changes
experimental/InjectiveResolutions/src/InjectiveResolutions.jl Outdated
##Let W = k{Q\(a + F - Q)} and let H be a hyperplane containing F as a subset.
##Calculates generators of k{(a + H_+^°)\cap Q}.
##INPUT: semigroup Q_P as a polyhedron
# zonotope G_Q of Q_P as in Lemma 3.10
Copy link
Member

Choose a reason for hiding this comment

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

Excellent: I really appreciate these references to specific lemmas and theorems in the code, that will surely be a great asset to people in the future!

One tiny request, though: it is not immediately clear to me to which book or article these reference -- presumably https://arxiv.org/abs/math/0309256 from the PR description? If so, it would be great if e.g. at the top of this file, or in the README (or both places) this link could appear to remove any ambiguity (also, once this PR is merged, someone might read the code w/o knowing which PR it came from, so they wouldn't about that arXiv link at all.

(long term we'd put this paper into our bib file, too, but no need to do that now unless you really want to for some reason.)

@wdecker
Copy link
Collaborator

wdecker commented Sep 17, 2024 via email

@tom111
Copy link
Author

tom111 commented Sep 17, 2024

Thanks everyone for the comments so far. I understand that there are many issues with the code as it stands now. We have a limited understanding of both stylistic conventions and inner workings of OSCAR. I think the detailed analysis, naming conventions, etc. can be deferred a little. Let me ask on a more strategic level:

Most of the time, we will work with modules over affine semigroup rings. Such a ring can be presented a quotient of a polynomial ring modulo a toric ideal and comes with different polyhedral and algebraic data attached to it. For example, it is graded by a finitely generated subsemigroup of ZZ^d (we always refer to this as 'Q'). This monoid lives inside a polyhedral cone, and the faces of that cone correspond 1:1 to Q-graded prime ideals. The data structures of our injective modules and local cohomlogy are often polyhedral in nature, which is why we are really hopeful that OSCAR is the right system to do this in.

Does it make sense to abstract such monoid algebras (some people say affine semigroup rings) in OSCAR before getting started?

At the moment, such a ring would be represented in OSCAR as a normal quotient ring. We will have R a polynomial ring, I a toric ideal and then our base_ring for most of what we do will be S = R/I (together with its multigrading, which is very important). In fact, the same question arises for the grading. Mathematically it is by a monoid Q, but we will (have to) use equivalence classes of vectors in ZZ^d instead, i.e. work in a presentation.

Now If J is an ideal of S and we construct the quotient S/J, then this is automatically turned into R/(I+J). I understand that this is what is needed internally, but for what I'm modeling, S/J is an S-module. That's the structure we want, e.g. compute the injective resolution of S/J as an S-module. Everything will be Q-graded S-modules from here on. So, for example, quotient_ring_as_module at the moment does not work in such a situation and Anna implemented it the way we need it (in the criticized function quotient_by_ideal). But part of me thinks that this should maybe be done in OSCAR. But then maybe we are just doing it wrongly and a different paradigm solves this.

Is this pull request the correct place to discuss such questions?

Another basic question: In this case, there is this one paper where we take all our notation from. Is it fair to assume (in the docs and comments) that people have read that paper?

@mjrodgers
Copy link
Collaborator

@micjoswig or other commutative algebra people, do you think you can say something about this?

@micjoswig
Copy link
Member

@micjoswig or other commutative algebra people, do you think you can say something about this?

Will do next Monday.

@lkastner
Copy link
Member

lkastner commented Oct 2, 2024 •
edited
Loading

Answer after discussing with @micjoswig and @flenzen:

Does it make sense to abstract such monoid algebras (some people say affine semigroup rings) in OSCAR before getting started?

Yes, because we have affine algebras, we suggest something like AffineSemigroupAlgebra as a wrapper around our existing affine algebra type together with the auxiliary polyhedral data you need.

Eventually this should tie in with our standard graded rings and modules. Once you reach this stage, please talk to @HechtiDerLachs .

At the moment, such a ring would be represented in OSCAR as a normal quotient ring. We will have R a polynomial ring, I a toric ideal and then our base_ring for most of what we do will be S = R/I (together with its multigrading, which is very important). In fact, the same question arises for the grading. Mathematically it is by a monoid Q, but we will (have to) use equivalence classes of vectors in ZZ^d instead, i.e. work in a presentation.

That should take care of this issue.

Now If J is an ideal of S and we construct the quotient S/J, then this is automatically turned into R/(I+J). I understand that this is what is needed internally, but for what I'm modeling, S/J is an S-module. That's the structure we want, e.g. compute the injective resolution of S/J as an S-module. Everything will be Q-graded S-modules from here on. So, for example, quotient_ring_as_module at the moment does not work in such a situation and Anna implemented it the way we need it (in the criticized function quotient_by_ideal). But part of me thinks that this should maybe be done in OSCAR. But then maybe we are just doing it wrongly and a different paradigm solves this.

We just looked at the documentation of quotient_ring_as_module and it seems currently impossible. Of course this suggestion makes sense.
Ping @thofma : Do you have a suggestion?

Is this pull request the correct place to discuss such questions?

Yes. You can also ask questions in the slack.

Another basic question: In this case, there is this one paper where we take all our notation from. Is it fair to assume (in the docs and comments) that people have read that paper?

Yes, please add an bib entry as described in our reference docs if necessary, and use this to references notation. Please note that the Miller Sturmfels book is already citable as MS05. Textbook references are preferred if viable.

@micjoswig
Copy link
Member

@micjoswig or other
Will do next Monday.

Sorry for the delay.

@tom111
Copy link
Author

tom111 commented Oct 2, 2024

Thanks everybody. We will get to work as soon as possible and proceed as suggested here.

@thofma
Copy link
Collaborator

thofma commented Oct 2, 2024 •
edited
Loading

Now If J is an ideal of S and we construct the quotient S/J, then this is automatically turned into R/(I+J). I understand that this is what is needed internally, but for what I'm modeling, S/J is an S-module. That's the structure we want, e.g. compute the injective resolution of S/J as an S-module. Everything will be Q-graded S-modules from here on. So, for example, quotient_ring_as_module at the moment does not work in such a situation and Anna implemented it the way we need it (in the criticized function quotient_by_ideal). But part of me thinks that this should maybe be done in OSCAR. But then maybe we are just doing it wrongly and a different paradigm solves this.

We just looked at the documentation of quotient_ring_as_module and it seems currently impossible. Of course this suggestion makes sense. Ping @thofma : Do you have a suggestion?

I had a quick glance at the examples in the PR here, but could not find an instance where this is used yet (maybe I looked at the wrong place). But from what you are saying, I think it is about the following situation (correct me if I am wrong):

julia> begin
         R_Q,(x,y) = graded_polynomial_ring(QQ,["x","y"])
         I = ideal(R_Q,[x^4,x^2*y^2,y^4])
         S, = quo(R_Q, I)
         J = ideal(S, [])
       end;

julia> quotient_by_ideal(J) # the method added here
(Graded submodule of S^1
[...]

julia> quotient_ring_as_module(J) # the method in Oscar
ERROR: MethodError: no method matching quotient_ring_as_module(::MPolyQuoIdeal{MPolyDecRingElem{QQFieldElem, QQMPolyRingElem}})

Closest candidates are:
  quotient_ring_as_module(::MPolyIdeal)
   @ Oscar ~/software/Oscar.jl/src/Modules/ModulesGraded.jl:2923
[...]

Is this correct? (I could not reproduce the "automatically turned into R/(I + J)" part.)

(Another difference would be that quotient_by_ideal also returns a map (but which is not used, at least in the examples yet).)

@wdecker
Copy link
Collaborator

wdecker commented Oct 2, 2024

May be I misunderstand the discussion here, but should we not extend the function quotient_ring_as_module by just allowing another type:

function quotient_ring_as_module(I::Union{MPolyIdeal, MPolyQuoIdeal})
R = base_ring(I)
F = is_graded(R) ? graded_free_module(R, 1) : free_module(R, 1)
e1 = F[1]
return quo_object(F, [x * e1 for x = gens(I)])
end

@wdecker wdecker mentioned this pull request Oct 3, 2024
Merged
@codecov Codecov
Copy link

codecov bot commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 0% with 383 lines in your changes missing coverage. Please review.

Project coverage is 84.21%. Comparing base (c6ceed4) to head (d15be9f).
Report is 81 commits behind head on master.

Files with missing lines Patch % Lines
...l/InjectiveResolutions/src/InjectiveResolutions.jl 0.00% 383 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4100      +/-   ##
==========================================
- Coverage   84.69%   84.21%   -0.49%     
==========================================
  Files         614      632      +18     
  Lines       83568    85282    +1714     
==========================================
+ Hits        70780    71822    +1042     
- Misses      12788    13460     +672
Files with missing lines Coverage Δ
...l/InjectiveResolutions/src/InjectiveResolutions.jl 0.00% <0.00%> (ø)

... and 157 files with indirect coverage changes

@annahofer00
Copy link

Thanks everyone! I just started working on this project again. I have introduced a type MonoidAlgebra as a struct with different fields for the auxiliary data. Is this the right way to go?

The rest of the code I am still working on and I don’t need any comments on it at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fingolfin fingolfin fingolfin left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
experimental Only changes experimental parts of the code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@tom111 @fingolfin @wdecker @mjrodgers @micjoswig @lkastner @thofma @annahofer00 @joschmitt