Skip to content

Add support for all vertical, well-quantized G4s that do not break the non-abelian gauge group#4446

Merged
HereAround merged 6 commits intooscar-system:masterfrom
HereAround:NewG4Fluxes
Jan 13, 2025
Merged

Add support for all vertical, well-quantized G4s that do not break the non-abelian gauge group#4446
HereAround merged 6 commits intooscar-system:masterfrom
HereAround:NewG4Fluxes

Conversation

@HereAround
Copy link
Copy Markdown
Member

@HereAround HereAround marked this pull request as draft January 10, 2025 15:20
@lgoettgens lgoettgens added the experimental Only changes experimental parts of the code label Jan 10, 2025
Copy link
Copy Markdown
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some random comments with small improvements to allocation reduction and type stability. There are many very similar opportunities to be improved that should be tackled at some point

Comment thread experimental/FTheoryTools/src/G4Fluxes/properties.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/properties.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/properties.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 78.01047% with 42 lines in your changes missing coverage. Please review.

Project coverage is 84.41%. Comparing base (a60b0d8) to head (c9639b2).
Report is 59 commits behind head on master.

Files with missing lines Patch % Lines
...al/FTheoryTools/src/G4Fluxes/special_attributes.jl 75.15% 39 Missing ⚠️
.../FTheoryTools/src/AbstractFTheoryModels/methods.jl 50.00% 2 Missing ⚠️
...rimental/FTheoryTools/src/G4Fluxes/constructors.jl 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4446      +/-   ##
==========================================
- Coverage   84.42%   84.41%   -0.02%     
==========================================
  Files         668      668              
  Lines       88363    88598     +235     
==========================================
+ Hits        74600    74788     +188     
- Misses      13763    13810      +47     
Files with missing lines Coverage Δ
...perimental/FTheoryTools/src/G4Fluxes/attributes.jl 100.00% <ø> (ø)
...perimental/FTheoryTools/src/G4Fluxes/properties.jl 98.46% <100.00%> (+0.63%) ⬆️
...tal/FTheoryTools/src/HypersurfaceModels/methods.jl 100.00% <100.00%> (ø)
.../FTheoryTools/src/LiteratureModels/constructors.jl 95.08% <ø> (ø)
experimental/FTheoryTools/src/auxiliary.jl 76.00% <100.00%> (ø)
experimental/FTheoryTools/test/tate_models.jl 100.00% <ø> (ø)
...rimental/FTheoryTools/src/G4Fluxes/constructors.jl 75.47% <83.33%> (+20.15%) ⬆️
.../FTheoryTools/src/AbstractFTheoryModels/methods.jl 78.00% <50.00%> (+0.06%) ⬆️
...al/FTheoryTools/src/G4Fluxes/special_attributes.jl 78.12% <75.15%> (-2.88%) ⬇️

... and 7 files with indirect coverage changes

@HereAround
Copy link
Copy Markdown
Member Author

Some random comments with small improvements to allocation reduction and type stability. There are many very similar opportunities to be improved that should be tackled at some point

Thank you. I have taken your points into account. Ideally, 4914e16 should improve the code efficiency. But I am certain that there is a LOT more that could be done - in a separate PR.

@HereAround HereAround marked this pull request as ready for review January 10, 2025 20:17
@HereAround
Copy link
Copy Markdown
Member Author

With that being said @apturner and @emikelsons , this PR should be ready for review.

@HereAround
Copy link
Copy Markdown
Member Author

HereAround commented Jan 11, 2025

I just took the liberty to push changes that hopefully will reduce the number of allocations in the computation of singular loci. A bit more tricky - postponed to the next PR: #4453

Comment thread experimental/FTheoryTools/src/G4Fluxes/constructors.jl
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl Outdated
Comment thread experimental/FTheoryTools/src/G4Fluxes/special_attributes.jl
HereAround and others added 2 commits January 13, 2025 13:10
Co-authored-by: Miķelis Emīls Miķelsons <mikelis.emils.mikelsons@gmail.com>
@HereAround
Copy link
Copy Markdown
Member Author

@emikelsons Thank you for spotting all those mistakes. Great catches! I just tried to fix those. Let us see if the tests succeed.

Copy link
Copy Markdown
Collaborator

@emikelsons emikelsons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@HereAround HereAround merged commit 301bc4f into oscar-system:master Jan 13, 2025
@HereAround HereAround deleted the NewG4Fluxes branch January 13, 2025 17:48
@aaruni96 aaruni96 added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label Jan 30, 2025
@fingolfin fingolfin changed the title [FTheoryTools] Add support for all vertical, well-quantized G4s that do not break the non-Abelian gauge group Add support for all vertical, well-quantized G4s that do not break the non-Abelian gauge group Feb 27, 2025
@fingolfin fingolfin changed the title Add support for all vertical, well-quantized G4s that do not break the non-Abelian gauge group Add support for all vertical, well-quantized G4s that do not break the non-abelian gauge group Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request experimental Only changes experimental parts of the code release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: FTheoryTools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants