experimental/GModule: collect AA/Nemo/Hecke stuff in Misc.jl
#3584
Conversation
Move code to `Misc.jl` that belongs to AbstractAlgebra.jl, Nemo.jl, or Hecke.jl. Once we agree that this code should be moved to these packages, the code should be added there, and as soon as it becomes available in Oscar, it can be removed from `Misc.jl`.
experimental/GModule/Brueckner.jl
Outdated
| @assert is_free(A) && is_free(B) | ||
| return hom(B, A, transpose(matrix(h))) | ||
| end | ||
|
|
||
| function coimage(h::Map) |
| @@ -299,10 +263,6 @@ function invariant_lattice_classes(M::GModule{<:Oscar.GAPGroup, <:AbstractAlgebr | |||
| return invariant_lattice_classes(MZ) | |||
| end | |||
|
|
|||
| function Oscar.pseudo_inv(h::Generic.ModuleHomomorphism) | |||
There was a problem hiding this comment.
For now, but eventually, this will have to be done differently as I'd like to have inv(inv()) to be the identity
| function Oscar.pseudo_inv(h::Generic.ModuleHomomorphism) | ||
| return MapFromFunc(codomain(h), domain(h), x->preimage(h, x)) | ||
| end | ||
|
|
||
| function _hom(f::Map{<:AbstractAlgebra.FPModule{T}, <:AbstractAlgebra.FPModule{T}}) where T |
There was a problem hiding this comment.
is this still used? It feels like either it should move or be deleted
There was a problem hiding this comment.
_hom is used just once, a few lines more down, in order to abbreviate the code.
If we move it, we should call it hom (?), and document what it is expected to do:
Take a map, assume that this map is linear, and return the corresponding "linear-map-by-images".
(In the current situation, I think it would be better to do this directly, by mapping generators under the various maps, instead of first creating several intermediate maps; but this would look less elegant.)
In this sense, deleting _hom is perhaps a good idea.
The problem is that the |
|
On Thu, Apr 11, 2024 at 07:50:24AM -0700, Thomas Breuer wrote:
@ThomasBreuer commented on this pull request.
> @@ -299,10 +263,6 @@ function invariant_lattice_classes(M::GModule{<:Oscar.GAPGroup, <:AbstractAlgebr
return invariant_lattice_classes(MZ)
end
-function Oscar.pseudo_inv(h::Generic.ModuleHomomorphism)
- return MapFromFunc(codomain(h), domain(h), x->preimage(h, x))
-end
-
function _hom(f::Map{<:AbstractAlgebra.FPModule{T}, <:AbstractAlgebra.FPModule{T}}) where T
`_hom` is used just once, a few lines more down, in order to abbreviate the code.
If we move it, we should call it `hom` (?), and document what it is expected to do:
Take a map, assume that this map is linear, and return the corresponding "linear-map-by-images".
(In the current situation, I think it would be better to do this directly, by mapping generators under the various maps, instead of first creating several intermediate maps; but this would look less elegant.)
In this sense, deleting `_hom` is perhaps a good idea.
Then go ahead!
…
--
Reply to this email directly or view it on GitHub:
#3584 (comment)
You are receiving this because your review was requested.
Message ID: ***@***.***>
|
|
Further changes e.g. to |
In
experimental/GModule, move code toMisc.jlthat belongs to AbstractAlgebra.jl, Nemo.jl, or Hecke.jl.Once we agree that this code should be moved to these packages, the code should be added there,
and as soon as it becomes available in Oscar, it can be removed from
Misc.jl.