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

[FTheoryTools] Overhaul base-independent models #2470

Merged
merged 10 commits into from Jul 13, 2023

Conversation

HereAround
Copy link
Member

@HereAround HereAround commented Jun 14, 2023

I split the part of the hypersurface model PR (#2382) off that will likely not run yet (and which I will likely not have the time to fix in the upcoming days). In this way, the original PR can now be merged shortly. This then allows @apturner to continue working on the F-Theory Tools.

@HereAround HereAround marked this pull request as draft June 14, 2023 08:33
@HereAround HereAround changed the title Hypersurface model II [FTheoryTools] Hypersurface model II Jun 14, 2023
@HereAround HereAround changed the title [FTheoryTools] Hypersurface model II [FTheoryTools] Overhaul base-independent models Jun 14, 2023
@HereAround HereAround force-pushed the HypersurfaceModel-II branch 4 times, most recently from 8add179 to aa6c978 Compare July 2, 2023 14:14
@codecov
Copy link

codecov bot commented Jul 2, 2023

Codecov Report

Merging #2470 (0cfa016) into master (d25569a) will increase coverage by 0.06%.
The diff coverage is 54.32%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2470      +/-   ##
==========================================
+ Coverage   72.76%   72.83%   +0.06%     
==========================================
  Files         412      412              
  Lines       55305    55410     +105     
==========================================
+ Hits        40245    40359     +114     
+ Misses      15060    15051       -9     
Impacted Files Coverage Δ
.../FTheoryTools/src/HypersurfaceModels/attributes.jl 0.00% <0.00%> (ø)
...TheoryTools/src/HypersurfaceModels/constructors.jl 0.00% <0.00%> (ø)
...tal/FTheoryTools/src/HypersurfaceModels/methods.jl 0.00% <0.00%> (ø)
...al/FTheoryTools/src/LiteratureModels/attributes.jl 0.00% <ø> (ø)
.../FTheoryTools/src/LiteratureModels/constructors.jl 0.00% <0.00%> (ø)
...ental/FTheoryTools/src/LiteratureModels/methods.jl 0.00% <ø> (ø)
...rimental/FTheoryTools/src/TateModels/attributes.jl 97.29% <ø> (ø)
...xperimental/FTheoryTools/src/TateModels/methods.jl 0.00% <ø> (ø)
...rimental/FTheoryTools/src/TateModels/properties.jl 100.00% <ø> (ø)
...l/FTheoryTools/src/WeierstrassModels/attributes.jl 97.43% <ø> (ø)
... and 5 more

... and 5 files with indirect coverage changes

@HereAround HereAround force-pushed the HypersurfaceModel-II branch 5 times, most recently from d9eccee to 9dc099f Compare July 4, 2023 13:11
@HereAround
Copy link
Member Author

HereAround commented Jul 4, 2023

In #2496, I am proposing an implementation of the most general star subdivision (following the toric geometry book by Cox Little Schenk, chapter 11). This was needed to get this PR running. I am now confident, that the tests for this PR shall pass or will do so shortly.

For my convenience (to make the necessary rebase as painless as possible), this PR contains the changes in #2496. Consequently, this other PR should be discussed first...

[I understand that the current failures in the nightly tests are not related to the changes in this PR.]

@HereAround HereAround marked this pull request as ready for review July 4, 2023 21:22
@HereAround HereAround force-pushed the HypersurfaceModel-II branch 6 times, most recently from b64728c to 4b35b2d Compare July 11, 2023 08:17
@HereAround
Copy link
Member Author

As long as features are concerned, I have just completed the overhaul of the base independent models. This includes:

  • Other auxiliary base spaces (by means of the most general star subdivision for toric spaces).
  • Have Kbar as homogeneous coordinate of the auxiliary base space.
  • Consistent order of the input variables of models over auxiliary base spaces. Information about the base space comes in first (base variables/ring, grading, dimension), fiber information second (special sections, fiber ambient space...).

Note that this also allowed me to switch on a test, that I initially had to switch off for this PR to work. In this particular test, initially no star triangulation could be found. By adding Kbar as base variable and using the most general star subdivisions for fans (as defined in CLS11), this is now working.

I suggest to wait for the tests to go green. Then this should be ready for review/merge.

@apturner What do you think?

@HereAround HereAround force-pushed the HypersurfaceModel-II branch 3 times, most recently from 924e6ac to 5235f8a Compare July 11, 2023 23:50
@HereAround HereAround force-pushed the HypersurfaceModel-II branch 2 times, most recently from 79eb43d to e62197d Compare July 12, 2023 04:15
@HereAround
Copy link
Member Author

I noticed that the doctests time out. The reason is the following:

julia> m = literature_model(arxiv_id = "1507.05954", equation = "A.1")
Weierstrass model over a not fully specified base -- U(1)xU(1) Weierstrass model based on arXiv paper 1507.05954 Eq. (A.1)

This model has an auxiliary base grading:

"auxiliary_base_grading": [
            [6, 4, 2, 3, 1, 0, 0, 0, 0, -1, -1, -1],
            [-3, -2, -1, -1, 0, 1, 0, 0, 0, 1, 1, 1],
            [-2, -1, 0, -1, 0, 0, 1, 0, 0, 1, 0, 0],
            [-2, -1, 0, -1, 0, 0, 0, 1, 0, 0, 1, 0],
            [-2, -1, 0, -1, 0, 0, 0, 0, 1, 0, 0, 1]
        ],

This means, the attempt to construct an auxiliary base space result in the triangulation of a 5 dimensional polytope with 12 vertices. That is excessive and, based on the above observation, leads to time outs of the doctests after 150 minutes. So this is no good for a doctest. Even more, this is likely not something that a user could hope to use...

I would like this PR to complete and be merged at some point (I have been working on these points since Andrew and I met in Kaiserslautern in May...). Hence, I have taken the liberty to just removed the calls and the tests based on this model.

Let us discuss how we can make this model usable again in another PR. One possibility would be to issue the triangulation on a cluster and save the result in the json file, so that one merely has to construct the toric space. Or possibly one can directly save the toric space. The question of course remains if such a triangulation will be at all successful when performed (on a cluster). That I cannot tell...

@HereAround
Copy link
Member Author

Generally speaking, it might be worth to save a triangulated base space in the json files. We should expect that this is a very time consuming operation. So users of the data base should not be punished by this...

Copy link
Collaborator

@apturner apturner left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for all the effort @HereAround!

@HereAround
Copy link
Member Author

Thank you @apturner I will open issues to keep track of the remaining points that could not (yet) be addressed.

@HereAround HereAround merged commit 389cb6b into oscar-system:master Jul 13, 2023
15 checks passed
@HereAround HereAround deleted the HypersurfaceModel-II branch July 13, 2023 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants