Add support for generic modules over the integers and fields #4915
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4915 +/- ##
========================================
Coverage 84.88% 84.89%
========================================
Files 697 697
Lines 94040 94203 +163
========================================
+ Hits 79823 79970 +147
- Misses 14217 14233 +16
🚀 New features to boost your workflow:
|
jankoboehm
force-pushed
the
intfieldmodules
branch
from
82bff7b to
514d23c
Compare
docs/src/CommutativeAlgebra/ModulesOverMultivariateRings/free_modules.md
Outdated
Show resolved
Hide resolved
docs/src/CommutativeAlgebra/ModulesOverMultivariateRings/free_modules.md
Outdated
Show resolved
Hide resolved
jankoboehm
force-pushed
the
intfieldmodules
branch
5 times, most recently
from
4182ba5 to
34998f3
Compare
|
Touched upon during triage. @jankoboehm still wants to add one more functions and then this will be ready for review. Eventually @jankoboehm and @HechtiDerLachs will look into designing a solve context for the sparse setting. Currently this uses the dense version provided by @thofma and @joschmitt . But this is for another PR to come (and probably picks up some of the stuff in #3969 ). |
jankoboehm
force-pushed
the
intfieldmodules
branch
from
34998f3 to
461715b
Compare
| ``` | ||
|
|
||
| ```@docs | ||
| size(M::SubquoModule{T}) where {T<:Union{ZZRingElem, FieldElem}} |
There was a problem hiding this comment.
stupid question, but: should we use size for this, given that it has a specific meaning in Julia already?
This is why we used order for groups or finite fields instead of size, even though e.g. GAP uses Size primarily (with Order "just" an alias. That said, order is somewhat common as terminology for groups and finite field, but I don't think it is usual for e.g. modules?
If we decide size is a good fit, then perhaps we should ponder whether to also allow it for fields and groups as an alias to order, simply for user convenience / easier discoverability...
There was a problem hiding this comment.
I am also fine with order and it is a better fit. a Shall we change it?
There was a problem hiding this comment.
Of course there are also orders in number theory, so I think we should keep size as it is.
There was a problem hiding this comment.
Is this meanwhile settled?
| if task == :via_transform | ||
| std, _ = lift_std(M) | ||
| return coordinates_via_transform(a, std) | ||
| return coordinates_via_transform(a, std) |
There was a problem hiding this comment.
| return coordinates_via_transform(a, std) | |
| return coordinates_via_transform(a, std) |
|
Timings for the local cohomology test suite: With this PR: Test Summary: | Pass Total Time
first local cohomology example | 9 9 2m01.7s
Test Summary: | Pass Total Time
hartshorne example | 6 6 2m39.6s
Test Summary: | Pass Total Time
local cohomology of module | 1 1 58.1sOn master: julia> include("experimental/InjectiveResolutions/test/local_cohomology.jl")
Test Summary: | Pass Total Time
first local cohomology example | 9 9 2m05.5s
Test Summary: | Pass Total Time
hartshorne example | 6 6 2m46.1s
Test Summary: | Pass Total Time
local cohomology of module | 1 1 1m02.1sSo these changes seem to be fine from the point of view of performance. |
jankoboehm
force-pushed
the
intfieldmodules
branch
from
461715b to
e7f069e
Compare
jankoboehm
added
the
release notes: to be added
fingolfin
changed the title
fingolfin
added
release notes: use title
5 participants
Make the generic modules work over the integers and fields. Also singles out the abstraction layer for the backend of the generic modules. The respective functions have atomic in their name. In a second iteration, the implementations of the atomic functions will be moved to separate files, and documentation of the abstraction layer will be added.