Skip to content

param_names in model-specific closure test#347

Closed
vschutze-alt wants to merge 6 commits intomainfrom
param_names_runcard
Closed

param_names in model-specific closure test#347
vschutze-alt wants to merge 6 commits intomainfrom
param_names_runcard

Conversation

@vschutze-alt
Copy link
Collaborator

@vschutze-alt vschutze-alt commented May 25, 2025

Make sure that it is possible to include the parameter names in any order in the runcard when running a model-specific closure test.

…can be listed by name in the runcard for model-specific closure test
@codecov
Copy link

codecov bot commented May 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.55%. Comparing base (a0bfa28) to head (c507071).
⚠️ Report is 227 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
+ Coverage   94.52%   94.55%   +0.02%     
==========================================
  Files          28       28              
  Lines        1297     1304       +7     
==========================================
+ Hits         1226     1233       +7     
  Misses         71       71              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Copy link
Member

@comane comane left a comment

Choose a reason for hiding this comment

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

Thanks for the addition, I have a couple of comments.

@comane comane requested a review from LucaMantani May 26, 2025 14:19
@comane comane linked an issue May 28, 2025 that may be closed by this pull request
Checks that the required keys are included in closure_test_model_settings
and constructs a dictionary for the settings.
"""
known_keys = {"model", "fitted_flavours", "parameters"}
Copy link
Member

Choose a reason for hiding this comment

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

Why fitted_flavours?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the runcard I have been using to run model-specific closure tests I have the following:

closure_test_model_settings: model: les_houches_example fitted_flavours: [\Sigma, g, V, V3]

So that's why I thought that fitted_flavours is a necessary key in this case. If it is only optional, I can remove it from the parse rule.

Copy link
Member

Choose a reason for hiding this comment

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

But we have also removed_fitted flavours from the les houches PDF. Also, I am a bit confused with this, does it allow to use wmin? It seems to allow only these 3 keys now, and for wmin one has to specify wmin_settings @comane

Copy link
Member

Choose a reason for hiding this comment

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

why would you need to specify wmin_settings?
I am a bit confused since this is independent on the closure test but a key that is needed for any wmin fit no?

I also was looking to see whether I could find a wmin model specific closure test runcard but didnt, do you have one perhaps?

Copy link
Member

Choose a reason for hiding this comment

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

#280

There was an example in the PR. You have to specify it because you need to specify the settings to build the PDF

Copy link
Member

Choose a reason for hiding this comment

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

I mean the point of this model_specific closure test is that you can build PDFs from the installed models, so you have to be free to specify everything that is needed to build them, i.e. what the constructor of the model needs. See this:

def pdf_model_from_colibri_model(model_settings):

…_model_pdf and allow for any keys, as long as parameters and model are there. Adapt unit test accordingly
@vschutze-alt vschutze-alt changed the title add parse rule for closure_test_colibri_model_pdf so that parameters … param_names in model-specific closure test Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Parse rule for closure_test_colibri_model_pdf

3 participants