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

Bump dependencies #3667

Merged
merged 13 commits into from
May 4, 2024
Merged

Bump dependencies #3667

merged 13 commits into from
May 4, 2024

Conversation

lgoettgens
Copy link
Member

@lgoettgens lgoettgens commented May 1, 2024

@lgoettgens
Copy link
Member Author

I now added base_ring_type methods for many of the already existing base_ring methods. In algebraic geometry, I got a bit lost and skipped some. So let's hope that CI passes anyway.

@lgoettgens
Copy link
Member Author

I know of some other stuff that needs to be adjusted here. Possibly getting to it tomorrow

@fingolfin
Copy link
Member

Needs a rebase anyway

@fingolfin
Copy link
Member

It seems this does not yet have the QQBarField disambiguation method for Polymake code

@fingolfin
Copy link
Member

I merged master (hopefully resolved the conflicts in src/aliases.jl correctly) and added a QQBarFieldElem disambiguation.

However there is still a precompilation "warning" / error:

WARNING: Method definition hom(Hecke.FinGenAbGroup, Hecke.FinGenAbGroup, Array{var"#s685", 1} where var"#s685"<:(AbstractAlgebra.Map{Hecke.FinGenAbGroup, Hecke.FinGenAbGroup, S, T} where T where S)) in module Hecke at /Users/mhorn/.julia/packages/Hecke/56bDu/src/GrpAb/GrpAbFinGen.jl:990 overwritten in module Misc at /Users/mhorn/Projekte/OSCAR/Oscar.jl/experimental/GModule/Misc.jl:257.

Looking at the two methods, they indeed have identical signature but one implements a tensor produce and one a direct product?!

@fingolfin
Copy link
Member

(CC @ThomasBreuer and @fieker as they may know best what to do about the hom method in experimental/GModule/Misc.jl vs. that in Hecke)

@lgoettgens
Copy link
Member Author

lgoettgens commented May 2, 2024

(CC @ThomasBreuer and @fieker as they may know best what to do about the hom method in experimental/GModule/Misc.jl vs. that in Hecke)

This error is due to me renaming this from direct_sum to hom in spite of Nemocas/AbstractAlgebra.jl#1675 (comment). We could just merge both of these functions to a single one that decides which construction to use depending on the if G is a direct sum or tensor product.

See thofma/Hecke.jl#1483 for the proposal.

@thofma
Copy link
Collaborator

thofma commented May 2, 2024

I think it is problematic that we now have to write methods for objects of some T and then we have to distinguish between how they were constructed. It is rather error prone.

This will only get worse, since there are also hom(G, H, vector), where H is a direct product and vector homomorphisms of G into the single components of H (aka universal property).

Clearly if I intend to do hom(G, H, vector_of_morphisms), I just constructed G and H as say tensor products, and then hom_tensor_product(G, H, vector_of_morphisms) makes more sense I would say. (Or some name containing the word "tensor").

@lgoettgens
Copy link
Member Author

I think it is problematic that we now have to write methods for objects of some T and then we have to distinguish between how they were constructed. It is rather error prone.

This will only get worse, since there are also hom(G, H, vector), where H is a direct product and vector homomorphisms of G into the single components of H (aka universal property).

Clearly if I intend to do hom(G, H, vector_of_morphisms), I just constructed G and H as say tensor products, and then hom_tensor_product(G, H, vector_of_morphisms) makes more sense I would say. (Or some name containing the word "tensor").

That was exactly my point in Nemocas/AbstractAlgebra.jl#1675 (comment). Let's wait for @ThomasBreuer and @fieker to comment on this. In any way, the functions for FinGenAbGrp and FPModule should be named the same, so in the worst case we need to rename the thing in AA again.

@lgoettgens lgoettgens closed this May 2, 2024
@lgoettgens lgoettgens reopened this May 2, 2024
@thofma thofma mentioned this pull request May 2, 2024
9 tasks
@fieker
Copy link
Contributor

fieker commented May 2, 2024 via email

@lgoettgens lgoettgens force-pushed the lg/compatibility branch 2 times, most recently from 6f98ef9 to 5dac6b1 Compare May 2, 2024 14:47
@thofma
Copy link
Collaborator

thofma commented May 2, 2024

@lgoettgens did you disable the tests with the problematic hom things?

@lgoettgens
Copy link
Member Author

@lgoettgens did you disable the tests with the problematic hom things?

Yes, the GModule tests (exercising the "wrong" hom function) and the Aqua tests (failing because of method overwriting) are currently disabled to find everything else that needs to be adapted

@lgoettgens lgoettgens force-pushed the lg/compatibility branch 2 times, most recently from 38c806a to 551da26 Compare May 3, 2024 17:24
Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 83.12%. Comparing base (2df8126) to head (cd10f0a).
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3667      +/-   ##
==========================================
- Coverage   83.13%   83.12%   -0.01%     
==========================================
  Files         577      577              
  Lines       78347    78306      -41     
==========================================
- Hits        65130    65091      -39     
+ Misses      13217    13215       -2     
Files Coverage Δ
experimental/GModule/GModule.jl 59.36% <ø> (ø)
experimental/GModule/Misc.jl 44.69% <ø> (-6.19%) ⬇️
experimental/LieAlgebras/src/LieAlgebras.jl 100.00% <ø> (ø)
experimental/ModStd/src/ModStdQt.jl 57.29% <ø> (ø)
experimental/Schemes/CoveredProjectiveSchemes.jl 88.40% <ø> (ø)
experimental/Schemes/duValSing.jl 100.00% <ø> (ø)
src/Combinatorics/SimplicialComplexes.jl 94.95% <ø> (ø)
src/Modules/ModuleTypes.jl 78.90% <ø> (ø)
src/Modules/UngradedModules/FreeMod.jl 84.00% <ø> (ø)
src/Modules/UngradedModules/FreeModElem.jl 95.74% <ø> (ø)
... and 31 more

... and 7 files with indirect coverage changes

@thofma thofma marked this pull request as ready for review May 4, 2024 07:36
@thofma thofma closed this May 4, 2024
@thofma thofma reopened this May 4, 2024
@thofma thofma enabled auto-merge (squash) May 4, 2024 08:41
@thofma thofma merged commit c6300cf into oscar-system:master May 4, 2024
57 of 83 checks passed
@lgoettgens lgoettgens deleted the lg/compatibility branch May 4, 2024 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ambiguity in QQBarField conversion
4 participants