move code from Oscar.jl/experimental/GModule/Misc.jl here
#1456
Conversation
The idea is to add this code to Hecke.jl, and then to remove it from Oscar.jl. Once we agree what shall be really added here, tests and documentation has to be added. (I am quite sure that I have put certain parts into the wrong files, and that some functions have to be changed. For example: - The function `dual` belongs to Hecke.jl, but the method in question deals with objects from AbstractAlgebra.jl; should `dual` better get defined in AbstractAlgebra.jl? - `pseudo_inv` is defined in AbstractAlgebra.jl, the method in question uses `MapFromFunc`, which belongs to Hecke.jl; what is the relation between Hecke.jl's `MapFromFunc` and AbstractAlgebra.jl's `FunctionalMap`?)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1456 +/- ##
==========================================
- Coverage 75.21% 75.07% -0.14%
==========================================
Files 354 354
Lines 112712 112844 +132
==========================================
- Hits 84771 84720 -51
- Misses 27941 28124 +183
|
|
Can we get this merged? Or is there any serious blocker (perhaps @thofma @fieker @lgoettgens any thoughts on this? |
(in #1482 (comment) by @thofma) |
| For groups `G = prod G_i` and `H = prod H_i` as well as maps `V_i: G_i -> H_i`, | ||
| build the induced map from `G -> H`. | ||
| """ | ||
| function direct_sum(G::FinGenAbGroup, H::FinGenAbGroup, V::Vector{<:Map{FinGenAbGroup, FinGenAbGroup}}) |
If the things do not belong in Oscar, and they do not belong here, I don't understand why it is better to have them here. If there is an urgency to move this, at least remove the one export statement that is added. |
|
There is no urgency in this from my POV, it just seems prudent to move it along eventually... But I think @ThomasBreuer is busy working on other things right now, and hopefully can back to this once those other things are done. |
| @@ -293,6 +293,12 @@ function (A::FinGenAbGroup)(x::ZZMatrix) | |||
| return z | |||
| end | |||
|
|
|||
| function (A::FinGenAbGroup)(x::FinGenAbGroupElem) | |||
There was a problem hiding this comment.
@ThomasBreuer @fieker so this is the function we talked about this morning, which is the reason (or at least a reason) why the GModule tests are run in Oscar even when "exclude experimental" is active...
But this PR here is waiting for @ThomasBreuer to finish it up -- then we can make a Hecke release, adjust Oscar, and perhaps get rid of that hack in Oscar....
|
As a first step, the code could be moved from |
|
@lgoettgens I can move the code, but I do not catch the point. |
The benefit of first moving everything in Oscar to a central place and do discussions there is that the moving itself is a trivial action that does not need to involve that many people to cooperate, but it is just a simple copy-paste that e.g. Tommy can just do when the next breaking Hecke release is planned |
|
@lgoettgens O.k. |
The idea is to add this code to Hecke.jl,
and then to remove it from Oscar.jl.
Once we agree what shall be really added here,
tests and documentation has to be added.
(I am quite sure that I have put certain parts into the wrong files, and that some functions have to be changed.
For example:
dualbelongs to Hecke.jl, but the method in question deals with objects from AbstractAlgebra.jl; shoulddualbetter get defined in AbstractAlgebra.jl?pseudo_invis defined in AbstractAlgebra.jl, the method in question usesMapFromFunc, which belongs to Hecke.jl; what is the relation between Hecke.jl'sMapFromFuncand AbstractAlgebra.jl'sFunctionalMap?)