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.

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

extend iso_oscar_gap(FO::AnticNumberField) #2511

Merged
merged 3 commits into from Jul 4, 2023
Merged

extend iso_oscar_gap(FO::AnticNumberField) #2511

merged 3 commits into from Jul 4, 2023

Conversation

ThomasBreuer
Copy link
Member

to iso_oscar_gap(FO::NumField),
where the field can be simple or non-simple, absolute or non-absolute

(suggested by @wdecker)

to `iso_oscar_gap(FO::NumField)`,
where the field can be simple or non-simple, absolute or non-absolute
@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #2511 (0be6fec) into master (ef76991) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2511      +/-   ##
==========================================
+ Coverage   72.94%   73.00%   +0.06%     
==========================================
  Files         406      406              
  Lines       53962    54168     +206     
==========================================
+ Hits        39360    39544     +184     
- Misses      14602    14624      +22     
Impacted Files Coverage Δ
src/GAP/iso_oscar_gap.jl 98.55% <100.00%> (+1.06%) ⬆️

... and 23 files with indirect coverage changes

@thofma
Copy link
Collaborator

thofma commented Jul 4, 2023

Can we handle M with QQ/K/L/M?

@wdecker
Copy link
Collaborator

wdecker commented Jul 4, 2023

Thanks, Thomas!

@wdecker wdecker merged commit 1ef21a4 into oscar-system:master Jul 4, 2023
12 of 15 checks passed
@ThomasBreuer
Copy link
Member Author

@thofma Sorry, I do not understand the question.

Can we handle M with QQ/K/L/M?

@ThomasBreuer ThomasBreuer deleted the TB_iso_oscar_gap_number_field branch July 4, 2023 07:12
@thofma
Copy link
Collaborator

thofma commented Jul 4, 2023

This still does not cover arbitrary number fields. Is this on purpose or a limitation on the GAP side?

@ThomasBreuer
Copy link
Member Author

@thofma GAP knows only simple extensions. Thus iso_oscar_gap switches from a non-simple extension to a simple one on the Oscar side if necessary.
For simple extensions in characteristic zero, GAP can deal with base fields that are subfields of cyclotomic fields, or (recursively constructed) simple extensions of such fields. We know how to translate SimpleNumField{nf_elem} to GAP,
with a special treatment of SimpleNumField{QQFieldElem}.

If I understand you right then you propose to offer a general iso_oscar_gap(FO::SimpleNumField{T}) where T <: FieldElem, which would in principle work (recursively).
I can make these changes, but I am not sure how far we get with this approach on the GAP side.
There is no problem as long as we need only arithmetics with elements from such fields, or arithmetics in groups of matrices over these fields, as in @wdecker's example (which is now contained in test/GAP/iso_oscar_gap.jl).

On the other hand, such computations can be done also with elements from fields that are defined over QQ. That is, there are situations where one does not really need iso_oscar_gap for an Oscar object such that a GAP structure gets created that is analogous to the one in Oscar, in the sense that the base fields on the two sides correspond to each other etc.
For example, my first proposal to deal with the example had been to replace the non-simple field extension by a simple one, and to do the matrix group computations over that field. (This approach is in fact faster than using the extended iso_oscar_gap.)

@thofma
Copy link
Collaborator

thofma commented Jul 4, 2023

Yes, I wondered why we don't implement the recursive case directly. It is the identical code.

@ThomasBreuer
Copy link
Member Author

@thofma O.k., I will create a pull request for that.

@thofma
Copy link
Collaborator

thofma commented Jul 4, 2023

We could also "flatten" an arbitrary field K to an AnticNumberField, that is, a simple extension of QQ, by calling absolute_simple_field(K).

@ThomasBreuer
Copy link
Member Author

Yes, this is what I had tried to sketch above:
What do we really want from iso_oscar_gap?
If we only want to deal with elements from the field then it does not matter what the base field is on the GAP side.
But if one wants to have an "isomorphism" then the algebraic structures on both sides have to correspond to each other.
Currently I think that iso_oscar_gap promises such an isomorphism.
Thus the code that wants to delegate questions to GAP could apply absolute_simple_field if it knows that no proper isomorphism is needed and that the "flattened" version behaves better.
Perhaps the code that deals with matrix groups over number fields is a place where this should be done by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants