Improve the handling of matrix groups over number fields#4826
Improve the handling of matrix groups over number fields#4826fingolfin merged 2 commits intooscar-system:masterfrom
Conversation
| try | ||
| compute_order(G) | ||
| catch e | ||
| return false | ||
| end | ||
| return true |
There was a problem hiding this comment.
It would indeed be nicer to avoid the try/catch here, since it is unspecific: if some other error is thrown, then we won't recognize it and treat it as meaning "the group is infinite" even though it might be finite and the exception was due to a bug...
So perhaps _isomorphic_group_over_finite_field could be changed to return e.g. nothing to indicate the group is infinite, and then we roll with it? I.e. we could change isomorphic_group_over_finite_field to detect this return value; if so, it could immediately mark the group G as infinite. Otherwise, it stores the morphism as before. This way we basically get no overhead.
Alternatively we could change _isomorphic_group_over_finite_field to thrown an InfiniteOrderError (actually that might be a good idea to do anyway).
There was a problem hiding this comment.
I prefer the InfiniteOrderError variant.
Concerning overhead, the current setup must be changed then, since _isomorphic_group_over_finite_field(G) delegates to _isomorphic_group_over_finite_field(gens(G)), hence we do not have the object G in the situation where InfiniteOrderError(G) gets called, and we do not want to create it there anew.
Ah, and then one finds that experimental/MatrixGroups/ defines a matrix_group method that is not compatible with what we want. Currently this method is not exported to Oscar, but once it gets exported we are in trouble.
|
I've restarted CI here, given that now GAP 4.15.1 is used by Oscar via GAP.jl 0.16. |
|
I would hope that with this PR, we can also get rid of the current manual order computations in |
|
Ping @ThomasBreuer |
(These changes require the GAP changes from gap-system/gap/pull/5970.) The idea is to set the GAP filter `MayBeHandledByNiceMonomorphism` in the `GapObj` of an Oscar matrix group over a number field, and to store the corresponding Oscar group in the attribute `JuliaData` of the GAP group, in order to give GAP access to the Oscar group. As soon as the Oscar group asks its `GapObj` for a computation that may benefit from a "nice monomorphism", the GAP group is asked for its `IsHandledByNiceMonomorphism` value. This question triggers the `is_finite` computation for the Oscar group. If the answer is `false` then "nice monomorphisms" do not help, and if the answer is `true` then an isomorphism from the Oscar group to a matrix group over a finite field has been computed, which is used to create a "nice monomorphism" for the GAP group. For that, a `IsHandledByNiceMonomorphism` method for the GAP group is needed, which makes an `is_finite` test. Note that up to now, `is_finite` for matrix groups simply delegated the work to GAP, which eventually computes a "nice monomorphism" in a different way, and then the reduction provided by Oscar does not help. Now we use a special `is_finite` method that triggers the computation of the reduction to a matrix group over a finite field. Since the reduction functions throw an error exception if (and only if) the matrix group in question is not finite, we use `try`/`catch`. Perhaps this setup should better be changed, in the sense that the return value of `_isomorphic_group_over_finite_field` indicates whether the given matrices generate a finite group or not.
- `_isomorphic_group_over_finite_field(gens(G))` now does not throw an error if the group generated by the given matrices is infinite. Instead, the function now returns `false, nothing` in this case. For finite groups, `true` plus the original return value is returned. This way, we can detect whether the group generated by the given matrices is finite or not. - `_isomorphic_group_over_finite_field(G)` now checks the flag in the result of `_isomorphic_group_over_finite_field(gens(G))`: If the group is finite, the behaviour is unchanged. If the group is infinite, an `InfiniteOrderError` is thrown. - `Oscar.MatrixGroups.matrix_group` was adjusted to the above changes.
11a422a to
edd3713
Compare
|
Concerning |
These changes are intended to solve the problems described in the discussion #4749.
(They require the GAP changes from gap-system/gap/pull/5970.)
The idea is to set the GAP filter
MayBeHandledByNiceMonomorphismin theGapObjof an Oscar matrix group over a number field, and to store the corresponding Oscar group in the attributeJuliaDataof the GAP group, in order to give GAP access to the Oscar group.As soon as the Oscar group asks its
GapObjfor a computation that may benefit from a "nice monomorphism",the GAP group is asked for its
IsHandledByNiceMonomorphismvalue. This question triggers theis_finitecomputation for the Oscar group. If the answer isfalsethen "nice monomorphisms" do not help, and if the answer istruethen an isomorphism from the Oscar group to a matrix group over a finite field has been computed, which is used to create a "nice monomorphism" for the GAP group.For that, a
IsHandledByNiceMonomorphismmethod for the GAP group is needed, which makes anis_finitetest.Note that up to now,
is_finitefor matrix groups simply delegated the work to GAP,which eventually computes a "nice monomorphism" in a different way, and then the reduction provided by Oscar does not help. Now we use a special
is_finitemethod that triggers the computation of the reduction to a matrix group over a finite field. Since the reduction functions throw an error exception if (and only if) the matrix group in question is not finite, we usetry/catch. Perhaps this setup should better be changed, in the sense that the return value of_isomorphic_group_over_finite_fieldindicates whether the given matrices generate a finite group or not.