[FTheoryTools] Added more checks for G4 fluxes #4044

emikelsons · 2024-08-26T11:40:38Z

Added verticality check for G4 fluxes
Changed zero_section method to now return a cohomology class in the toric ambient space. Old version of the method is now under zero_section_coordinates(this is outdated, see below)
Added D3-tadpole cancellation check for G4 fluxes.

codecov · 2024-08-26T12:59:19Z

Codecov Report

Attention: Patch coverage is 97.77778% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.61%. Comparing base (f777526) to head (30ae7fd).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
...perimental/FTheoryTools/src/G4Fluxes/properties.jl	96.87%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4044   +/-   ##
=======================================
  Coverage   84.61%   84.61%           
=======================================
  Files         612      612           
  Lines       83156    83200   +44     
=======================================
+ Hits        70359    70402   +43     
- Misses      12797    12798    +1

Files with missing lines	Coverage Δ
...heoryTools/src/AbstractFTheoryModels/attributes.jl	`99.59% <100.00%> (+<0.01%)`	⬆️
.../FTheoryTools/src/AbstractFTheoryModels/methods.jl	`74.92% <100.00%> (+0.53%)`	⬆️
...heoryTools/src/AbstractFTheoryModels/properties.jl	`100.00% <100.00%> (ø)`
.../FTheoryTools/src/LiteratureModels/constructors.jl	`93.81% <100.00%> (+0.04%)`	⬆️
...perimental/FTheoryTools/src/G4Fluxes/properties.jl	`97.82% <96.87%> (-2.18%)`	⬇️

HereAround · 2024-08-29T09:37:35Z

I have just merged #3979. It seems this leads to to merge conflicts (see below the CI test results). @emikelsons , could you please rebase to the current master.

HereAround · 2024-08-29T09:38:55Z

I would think that the two test failures are unrelated to your changes, but we could see this more clearly once the tests are executed with your PR rebase to the current master.

experimental/FTheoryTools/src/G4Fluxes/properties.jl

HereAround · 2024-08-29T11:33:14Z

experimental/FTheoryTools/src/G4Fluxes/properties.jl

-      numb = integrate(cohomology_class(ambient_space(m), twist_g4 * cy * c_ds[i] * c_ds[j]); check = false)
+      numb = integrate(cohomology_class(ambient_space(m), twist_g4 * c_ds[i] * c_ds[j] * cy); check = false)


I suppose this an unrelated change? Is it critical?

Not really critical. This happened as a result of some testing, but I left it as it made sense to me to have the hyperplane class at the end so that the expression is more readily comparable to the doc description.

experimental/FTheoryTools/src/G4Fluxes/properties.jl

HereAround · 2024-08-29T11:35:32Z

experimental/FTheoryTools/src/LiteratureModels/Models/model1903_00009.json

+    "hypersurface_equation": "s1*e1^2*e2^2*e3*e4^4*u^3+s2*e1*e2^2*e3^2*e4^2*u^2*v+s3*e2^2*e3^3*u*v^2+s5*e1^2*e2*e4^3*u^2*w+s6*e1*e2*e3*e4*u*v*w+s9*e1*v*w^2",
+    "zero_section": "v"


Is the zero section for this model really v? I do not recall it at the top of my head, just want to double-check. (Probably you already did that.)

I saw it mentioned in the paper, it passed all of the tests and that was good enough for me

lgoettgens

I found two missing ensure_artifacts_installed. Please make sure to always include this line in doctests accessing the QSMDB: As the order of doctests in non-deterministic, any doctest might be the first one accessing the QSMDB. And if this setup is then missing, it will make the doctest fail. See e.g. of such an instance.

experimental/FTheoryTools/src/AbstractFTheoryModels/attributes.jl

experimental/FTheoryTools/src/G4Fluxes/properties.jl

HereAround · 2024-08-29T13:29:26Z

I found two missing ensure_artifacts_installed. Please make sure to always include this line in doctests accessing the QSMDB: As the order of doctests in non-deterministic, any doctest might be the first one accessing the QSMDB. And if this setup is then missing, it will make the doctest fail. See e.g. of such an instance.

Thank you for catching this @lgoettgens !

experimental/FTheoryTools/src/AbstractFTheoryModels/attributes.jl

HereAround · 2024-09-04T14:13:30Z

Thank you for working on this @emikelsons . This looks good to me!

HereAround

I approve this PR modulo the above changes to the used references.

@emikelsons please adjust these references and ping me once you have done. Then this can be merged.

HereAround · 2024-09-04T14:15:58Z

experimental/FTheoryTools/src/G4Fluxes/properties.jl

@@ -63,7 +63,7 @@ end
 @doc raw"""
    passes_verticality_checks(gf::G4Flux)

-G4-fluxes are subject to verticality conditions described in [Lin16](@cite).
+G4-fluxes are subject to verticality conditions described in [Lin16](@cite) and [KMW12](@cite).


I would suggest to link to Timo Weigand F-theory TASI lectures instead. Maybe in fact, replace those two references with this?

@apturner also suggests to reference instead of Ling's thesis the historically first paper that ever talked about these conditions. For this, just check which papers the above Tasi lectures reference.

HereAround · 2024-09-04T14:17:53Z

docs/oscar_references.bib

+@Article{KMW12,
+  author        = {Krause, Sven and Mayrhofer, Christoph and Weigand, Timo},
+  title         = {Gauge fluxes in F-theory and type IIB orientifolds},
+  journal       = {Journal of High Energy Physics},
+  volume        = {2012},
+  number        = {8},
+  publisher     = {Springer Science and Business Media LLC},
+  year          = {2012},
+  month         = {8},
+  doi           = {10.1007/jhep08(2012)119},
+  eprint        = {1202.3138}
+}
+


Presumably we do not need this reference? See comment below.

HereAround · 2024-09-04T14:18:25Z

experimental/FTheoryTools/src/AbstractFTheoryModels/attributes.jl

@@ -1245,7 +1248,7 @@ function euler_characteristic(m::AbstractFTheoryModel; check::Bool = true)

  # Check if the answer is known
  if has_attribute(m, :euler_characteristic)
-    return get_attribute(m, :euler_characteristic)::CohomologyClass
+    return get_attribute(m, :euler_characteristic)::Int


This is a great change. We need this!

HereAround · 2024-09-04T14:26:28Z

Another point raised just now is about the global naming scheme within FTheoryTools:

zero_section: Returns a vector with coordinates (currently strings, but at some point polynomials, rational functions or wilder beasts yet...).
zero_section_class: Return a cohomology class S in the toric ambient space s.t. S|Y4 is the cohomology class in Y4 of the zero section locus.

Bonus points:
When calling zero_section on a (partially) resolved model, we should get the zero section of the partially resolved model. Maybe we can obtain this by keeping track of it during the sequence of blowups? (If cannot be done easily within this PR, then let us open an issue/add this to our ToDo-List, so we do not forget about it.)

lgoettgens · 2024-09-05T08:33:06Z

experimental/FTheoryTools/src/AbstractFTheoryModels/attributes.jl

+julia> h = euler_characteristic(qsm_model; check = false)
+378
+


Suggested change

julia> h = euler_characteristic(qsm_model; check = false)

378

these three lines are already there right below

I added these lines twice on purpose to function as an additional test

Just to elaborate a bit: Once euler_characteristic is called for the 2nd time, it fetches the saved value of the attribute. This process failed in the past, because of the wrong type cast. Now this will be tested.

emikelsons · 2024-09-09T09:30:01Z

I have changed the references though I was not able to find the historically first paper that mentions the verticality conditions. I also reverted back to zero_section for the original zero_section and introduced the method zero_section_class that return the cohomology class. @HereAround @apturner

HereAround · 2024-09-09T09:38:30Z

The historically first paper regarding the verticality conditions seems to be https://arxiv.org/abs/1111.1232 (cf. https://arxiv.org/pdf/1806.01854, page 86, below equation 9.8).

HereAround

Thank you for working on this @emikelsons . This looks good to me now.

emikelsons marked this pull request as draft August 26, 2024 11:41

emikelsons force-pushed the MyChanges branch from aba7a48 to 4418dc7 Compare August 26, 2024 12:01

HereAround added the topic: FTheoryTools label Aug 26, 2024

emikelsons force-pushed the MyChanges branch 3 times, most recently from b47d807 to 3ac3986 Compare August 29, 2024 11:29

HereAround reviewed Aug 29, 2024

View reviewed changes

experimental/FTheoryTools/src/G4Fluxes/properties.jl Outdated Show resolved Hide resolved

HereAround reviewed Aug 29, 2024

View reviewed changes

experimental/FTheoryTools/src/G4Fluxes/properties.jl Outdated Show resolved Hide resolved

HereAround reviewed Aug 29, 2024

View reviewed changes

experimental/FTheoryTools/src/G4Fluxes/properties.jl Outdated Show resolved Hide resolved

HereAround reviewed Aug 29, 2024

View reviewed changes

emikelsons force-pushed the MyChanges branch from 3ac3986 to 785183e Compare August 29, 2024 11:55

lgoettgens reviewed Aug 29, 2024

View reviewed changes

experimental/FTheoryTools/src/AbstractFTheoryModels/attributes.jl Outdated Show resolved Hide resolved

experimental/FTheoryTools/src/G4Fluxes/properties.jl Outdated Show resolved Hide resolved

emikelsons force-pushed the MyChanges branch from 83800ea to 22f0fcb Compare August 31, 2024 08:03

emikelsons marked this pull request as ready for review August 31, 2024 10:17

lgoettgens reviewed Aug 31, 2024

View reviewed changes

experimental/FTheoryTools/src/AbstractFTheoryModels/attributes.jl Outdated Show resolved Hide resolved

emikelsons force-pushed the MyChanges branch 3 times, most recently from e8d3d2f to 35f9b27 Compare September 1, 2024 07:01

lgoettgens mentioned this pull request Sep 4, 2024

[FTheoryTools] Better type inference #4073

Merged

HereAround requested changes Sep 4, 2024

View reviewed changes

lgoettgens reviewed Sep 5, 2024

View reviewed changes

Added verticality checks for g4 fluxes and changed zero section methods

29d75bb

emikelsons force-pushed the MyChanges branch from fb72e59 to 38520cd Compare September 9, 2024 09:18

emikelsons force-pushed the MyChanges branch from 38520cd to 96eb31c Compare September 9, 2024 09:46

Added D3-tadpole cancellation check for G4-fluxes

79dc60c

emikelsons force-pushed the MyChanges branch from 96eb31c to 79dc60c Compare September 9, 2024 09:57

Added an additional reference

30ae7fd

HereAround approved these changes Sep 9, 2024

View reviewed changes

HereAround merged commit 6f7dca7 into oscar-system:master Sep 9, 2024
27 of 28 checks passed

emikelsons deleted the MyChanges branch September 10, 2024 07:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FTheoryTools] Added more checks for G4 fluxes #4044

[FTheoryTools] Added more checks for G4 fluxes #4044

emikelsons commented Aug 26, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading

HereAround commented Aug 29, 2024

HereAround commented Aug 29, 2024

HereAround Aug 29, 2024

emikelsons Aug 29, 2024

HereAround Aug 29, 2024

emikelsons Aug 29, 2024

lgoettgens left a comment

HereAround commented Aug 29, 2024

HereAround commented Sep 4, 2024

HereAround left a comment

HereAround Sep 4, 2024

HereAround Sep 4, 2024

HereAround Sep 4, 2024

HereAround Sep 4, 2024

HereAround commented Sep 4, 2024 •

edited

Loading

lgoettgens Sep 5, 2024

emikelsons Sep 5, 2024

HereAround Sep 9, 2024

emikelsons commented Sep 9, 2024

HereAround commented Sep 9, 2024

HereAround left a comment

		numb = integrate(cohomology_class(ambient_space(m), twist_g4 * cy * c_ds[i] * c_ds[j]); check = false)
		numb = integrate(cohomology_class(ambient_space(m), twist_g4 * c_ds[i] * c_ds[j] * cy); check = false)

		"hypersurface_equation": "s1e1^2e2^2e3e4^4u^3+s2e1e2^2e3^2e4^2u^2v+s3e2^2e3^3uv^2+s5e1^2e2e4^3u^2w+s6e1e2e3e4uvw+s9e1vw^2",
		"zero_section": "v"

		julia> h = euler_characteristic(qsm_model; check = false)
		378

[FTheoryTools] Added more checks for G4 fluxes #4044

[FTheoryTools] Added more checks for G4 fluxes #4044

Conversation

emikelsons commented Aug 26, 2024 • edited Loading

codecov bot commented Aug 26, 2024 • edited Loading

Codecov Report

HereAround commented Aug 29, 2024

HereAround commented Aug 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgoettgens left a comment

Choose a reason for hiding this comment

HereAround commented Aug 29, 2024

HereAround commented Sep 4, 2024

HereAround left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HereAround commented Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emikelsons commented Sep 9, 2024

HereAround commented Sep 9, 2024

HereAround left a comment

Choose a reason for hiding this comment

emikelsons commented Aug 26, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading

HereAround commented Sep 4, 2024 •

edited

Loading