Skip to content

FTheory: More documentation and related improvements#5071

Merged
HereAround merged 30 commits intomasterfrom
MoreDocumentation
Jul 7, 2025
Merged

FTheory: More documentation and related improvements#5071
HereAround merged 30 commits intomasterfrom
MoreDocumentation

Conversation

@HereAround
Copy link
Copy Markdown
Member

@HereAround HereAround commented Jul 4, 2025

Release notes:

  • #5071 Extended flexibility for HypersurfaceModel constructors.
  • #5071 Improved implementation of set_weierstrass_model(h::HypersurfaceModel).
  • #5071 Improved implementation of set_global_tate_model(h::HypersurfaceModel).
  • #5071 Breaking: Remove support for sample_toric_variety().
  • #5071 Breaking: Remove support for weierstrass_model(w::WeierstrassModel).

What is in this PR:

  • Restructure documentation text.
  • Improve documentation text and various doc strings.
  • Document hypersurface_equation for Weierstrass and global Tate models.
  • General code styling.
  • Remove/update comments.
  • Remove "# Examples" from all doc strings for consistency.
  • Added doc strings and examples to methods for which this was missing.
  • Remove all uses of literature models in doc string examples, that appear before literature models are introduced.
  • Introduce new and long desired constructors for hypersurface models. Namely, the provided twists need not be for the first two fiber ambient coordinates, but can now be specified for any two fiber ambient coordinates.
  • Improve implementation of set_weierstrass_model and set_global_tate_model for hypersurface models.
  • Stop support for function sample_toric_variety.
  • Stop support for the following function:
function weierstrass_model(w::WeierstrassModel)
  @vprint :FTheoryModelPrinter 0 "Weierstrass model provided, returning this very model.\n"
  return w
end

Preview of the overhauled documentation should become available at https://docs.oscar-system.org/previews/PR5071/Experimental/FTheoryTools/introduction/.

@HereAround HereAround added release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes topic: FTheoryTools documentation Improvements or additions to documentation optimization Simpler/more performant code or more/better tests labels Jul 4, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 4, 2025

Codecov Report

Attention: Patch coverage is 89.47368% with 8 lines in your changes missing coverage. Please review.

Project coverage is 84.89%. Comparing base (b1aab33) to head (80b821f).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
.../FTheoryTools/src/HypersurfaceModels/attributes.jl 60.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5071      +/-   ##
==========================================
- Coverage   84.90%   84.89%   -0.01%     
==========================================
  Files         706      706              
  Lines       95421    95482      +61     
==========================================
+ Hits        81017    81064      +47     
- Misses      14404    14418      +14     
Files with missing lines Coverage Δ
...heoryTools/src/AbstractFTheoryModels/attributes.jl 96.52% <ø> (ø)
.../FTheoryTools/src/AbstractFTheoryModels/methods.jl 66.96% <ø> (ø)
...al/FTheoryTools/src/FamilyOfG4Fluxes/attributes.jl 80.59% <ø> (ø)
.../FTheoryTools/src/FamilyOfG4Fluxes/constructors.jl 68.88% <ø> (ø)
...ental/FTheoryTools/src/FamilyOfG4Fluxes/methods.jl 96.96% <ø> (ø)
...Tools/src/FamilyOfG4Fluxes/special_constructors.jl 96.80% <ø> (ø)
...perimental/FTheoryTools/src/G4Fluxes/attributes.jl 63.75% <ø> (ø)
...xperimental/FTheoryTools/src/G4Fluxes/auxiliary.jl 94.60% <ø> (ø)
...rimental/FTheoryTools/src/G4Fluxes/constructors.jl 80.86% <ø> (ø)
...perimental/FTheoryTools/src/G4Fluxes/properties.jl 100.00% <ø> (ø)
... and 14 more

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@HereAround HereAround force-pushed the MoreDocumentation branch 4 times, most recently from 0dd09ed to bba0115 Compare July 4, 2025 12:53
@HereAround HereAround force-pushed the MoreDocumentation branch 3 times, most recently from ee85584 to 2e59fe4 Compare July 5, 2025 08:32
@HereAround HereAround force-pushed the MoreDocumentation branch 3 times, most recently from 4535b77 to 3ab40f4 Compare July 5, 2025 10:13
@HereAround HereAround force-pushed the MoreDocumentation branch from 3ab40f4 to dcecfaa Compare July 5, 2025 10:19
@HereAround HereAround force-pushed the MoreDocumentation branch from c0240e7 to a333f24 Compare July 5, 2025 23:01
@HereAround HereAround force-pushed the MoreDocumentation branch from a333f24 to 7823dcb Compare July 6, 2025 06:35
@HereAround HereAround added the extra-long Also run the extra-long tests during CI. label Jul 6, 2025
@HereAround HereAround closed this Jul 6, 2025
@HereAround HereAround reopened this Jul 6, 2025
@HereAround HereAround force-pushed the MoreDocumentation branch from 7bd48a5 to e882bc9 Compare July 6, 2025 12:13
@HereAround HereAround force-pushed the MoreDocumentation branch from e882bc9 to 76aa493 Compare July 6, 2025 12:31
@HereAround HereAround marked this pull request as ready for review July 6, 2025 22:47
@HereAround HereAround requested review from apturner and emikelsons July 6, 2025 22:47

Resolve an F-theory model by blowing up a locus in the ambient space.

# Examples
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are we removing # Examples, isn't the use of this subtitle above code samples common practice throughout the Oscar docs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

At least we (=FTheoryTools) were not consistent with this. To me it seemed that most of the FTheoryTools doc strings did not have the #Example and so I removed it.

(I also like simplicity. To me it seems clear that what follows below the description is an example. So why state the obvious? But this is definitely not too important and maybe even incorrect - perceptions can be different.)

I am happy to add this back if you feel this is preferred, but then adding it to all doc strings.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't really have a strong opinion either way and was just pointing out what I had noticed, but maybe it's best then to leave these changes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 and the tests seem to pass!

@HereAround HereAround merged commit 25e6c70 into master Jul 7, 2025
34 of 35 checks passed
@HereAround HereAround deleted the MoreDocumentation branch July 7, 2025 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation extra-long Also run the extra-long tests during CI. optimization Simpler/more performant code or more/better tests release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes topic: FTheoryTools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants