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] Support for literature model parameters #2569

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

apturner
Copy link
Collaborator

Adds support for literature model parameters, as well as associated attributes, properties, and documentation.

@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #2569 (739103d) into master (d0f7ea7) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2569      +/-   ##
==========================================
- Coverage   71.85%   71.82%   -0.04%     
==========================================
  Files         417      417              
  Lines       57320    57348      +28     
==========================================
+ Hits        41187    41189       +2     
- Misses      16133    16159      +26     
Impacted Files Coverage Δ
...al/FTheoryTools/src/LiteratureModels/attributes.jl 0.00% <0.00%> (ø)
...tal/FTheoryTools/src/LiteratureModels/auxiliary.jl 0.00% <0.00%> (ø)
.../FTheoryTools/src/LiteratureModels/constructors.jl 0.00% <0.00%> (ø)
...al/FTheoryTools/src/LiteratureModels/properties.jl 0.00% <0.00%> (ø)
...mental/FTheoryTools/src/TateModels/constructors.jl 72.15% <0.00%> (-1.88%) ⬇️
...FTheoryTools/src/WeierstrassModels/constructors.jl 66.25% <0.00%> (-1.70%) ⬇️

... and 4 files with indirect coverage changes

@apturner apturner added enhancement New feature or request topic: FTheoryTools labels Jul 19, 2023
@HereAround
Copy link
Member

Thank you @apturner for adding this functionality! This was the exact thing that I found missing a couple of days back. Nice!

However, unless I am mistaken, we do not have an example/test which constructs a literature model with model parameters. Please correct if I am mistaken.

If indeed correct, then I would suggest adding one, so that we can rest assured that the model parameters code works as intended.

@apturner
Copy link
Collaborator Author

We do actually have such examples; all 7 of the Lawrie/Schäfer-Nameki models have a parameter, and the related_literature_models and model_parameters functions that are added in this PR have doctests that construct these models, so this PR already tests that it works.

Copy link
Member

@HereAround HereAround left a comment

Choose a reason for hiding this comment

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

Thank you @apturner. Looks good to me.

Comment on lines +429 to +438
```jldoctest
julia> m = literature_model(arxiv_id = "1212.2949", equation = "3.2", model_parameters = Dict("k" => 5))
Assuming that the first row of the given grading is the grading under Kbar

Global Tate model over a not fully specified base -- SU(2k+1) Tate model with parameter values (k = 5) based on arXiv paper 1212.2949 Eq. (3.2)

julia> model_parameters(m)
Dict{String, Int64} with 1 entry:
"k" => 5
```
Copy link
Member

Choose a reason for hiding this comment

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

Very good. This here is exactly what I was looking for earlier, but somehow must have overlooked this test. Nice!

@HereAround HereAround merged commit 5ff0130 into oscar-system:master Jul 20, 2023
15 checks passed
@apturner apturner deleted the litModelParams branch August 8, 2023 15:25
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.

2 participants