From 3053da5111dc85aac5a72e6afe6b474bc1f6e22a Mon Sep 17 00:00:00 2001 From: vschutze-alt Date: Sun, 25 May 2025 22:30:11 +0100 Subject: [PATCH 1/6] add parse rule for closure_test_colibri_model_pdf so that parameters can be listed by name in the runcard for model-specific closure test --- colibri/closure_test.py | 2 +- colibri/config.py | 23 +++++++++++++++++++++++ colibri/tests/test_closure_test.py | 10 ++++++++-- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/colibri/closure_test.py b/colibri/closure_test.py index f7b488ba7..f27fc8fb5 100644 --- a/colibri/closure_test.py +++ b/colibri/closure_test.py @@ -73,7 +73,7 @@ def closure_test_colibri_model_pdf(closure_test_model_settings, FIT_XGRID): # Compute the pdf grid pdf_grid_func = pdf_model.grid_values_func(FIT_XGRID) - params = jnp.array(closure_test_model_settings["parameters"]) + params = jnp.array(list(closure_test_model_settings["parameters"].values())) pdf_grid = pdf_grid_func(params) return pdf_grid diff --git a/colibri/config.py b/colibri/config.py index d3495c770..827ddaeb9 100644 --- a/colibri/config.py +++ b/colibri/config.py @@ -521,3 +521,26 @@ def produce_pdf_model(self): Returns None as the pdf_model is not used in the colibri module. """ return None + + def parse_closure_test_colibri_model_pdf(self, settings): + known_keys = {"model", "fitted_flavours", "parameters"} + + kdiff = settings.keys() - known_keys + for k in kdiff: + log.warning( + ConfigError( + f"Key '{k}' in closure_test_model_settings not known.", + k, + known_keys, + ) + ) + + # Now construct the closure_test_model_settings dictionary, + # checking the parameter combinations are valid + closure_test_model_settings = {} + + closure_test_model_settings["model"] = settings.get("model") + closure_test_model_settings["fitted_flavours"] = settings.get("fitted_flavours") + closure_test_model_settings["parameters"] = settings.get("parameters") + + return closure_test_model_settings diff --git a/colibri/tests/test_closure_test.py b/colibri/tests/test_closure_test.py index 2ae90a690..8d67398fb 100644 --- a/colibri/tests/test_closure_test.py +++ b/colibri/tests/test_closure_test.py @@ -76,7 +76,10 @@ def test_closure_test_pdf_grid_with_colibri_model( # Mock the pdf model mock_pdf_model_from_colibri_model.return_value = mock_colibri_model - settings = {"parameters": [1, 2, 3], "model": "test_model"} + settings = { + "parameters": {"param1": 1, "param2": 2, "param3": 3}, + "model": "test_model", + } grid = closure_test_pdf_grid( "colibri_model", sample_xgrid, closure_test_model_settings=settings @@ -109,7 +112,10 @@ def test_closure_test_colibri_model_pdf( # Mock the pdf model mock_pdf_model_from_colibri_model.return_value = mock_colibri_model - settings = {"parameters": [1, 2, 3], "model": "test_model"} + settings = { + "parameters": {"param1": 1, "param2": 2, "param3": 3}, + "model": "test_model", + } pdf_grid = closure_test_colibri_model_pdf(settings, sample_xgrid) assert pdf_grid.shape == (3, 5) From 72c7b9863c3b6fbedaf72dd39fe790431f2eb847 Mon Sep 17 00:00:00 2001 From: vschutze-alt Date: Sun, 25 May 2025 23:18:52 +0100 Subject: [PATCH 2/6] add unit test for parse_closure_test_colibri_model_pdf --- colibri/tests/test_config.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/colibri/tests/test_config.py b/colibri/tests/test_config.py index 790be2b5c..bb6b75e2a 100644 --- a/colibri/tests/test_config.py +++ b/colibri/tests/test_config.py @@ -418,3 +418,33 @@ def test_produce_commondata_tuple(): closure_test_level = 2 with pytest.raises(ConfigError): BASE_CONFIG.produce_commondata_tuple(closure_test_level) + + + +@patch("colibri.config.log.warning") +def test_parse_closure_test_colibri_model_pdf(mock_warning): + # Mock the pdf model + + settings = { + "parameters": {"param1": 1, "param2": 2}, + "model": "test_model", + "fitted_flavours": ["T3", "T8"], + "unknown_key": "should_warn", + } + + # Call the method + result = BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings) + + # Assert the result is as expected + expected = { + "model": "test_model", + "fitted_flavours": ["T3", "T8"], + "parameters": {"param1": 1, "param2": 2}, + } + + assert result == expected + + # Check that the warning was called for the unknown key + mock_warning.assert_called_once() + args, _ = mock_warning.call_args + assert isinstance(args[0], ConfigError) \ No newline at end of file From fb39016b85d3acdf14b406d78560da14eb5601e2 Mon Sep 17 00:00:00 2001 From: vschutze-alt Date: Sun, 25 May 2025 23:19:21 +0100 Subject: [PATCH 3/6] add unit test for parse_closure_test_colibri_model_pdf - reformated --- colibri/tests/test_config.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/colibri/tests/test_config.py b/colibri/tests/test_config.py index bb6b75e2a..39a05078e 100644 --- a/colibri/tests/test_config.py +++ b/colibri/tests/test_config.py @@ -420,7 +420,6 @@ def test_produce_commondata_tuple(): BASE_CONFIG.produce_commondata_tuple(closure_test_level) - @patch("colibri.config.log.warning") def test_parse_closure_test_colibri_model_pdf(mock_warning): # Mock the pdf model @@ -432,7 +431,7 @@ def test_parse_closure_test_colibri_model_pdf(mock_warning): "unknown_key": "should_warn", } - # Call the method + # Call the method result = BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings) # Assert the result is as expected @@ -447,4 +446,4 @@ def test_parse_closure_test_colibri_model_pdf(mock_warning): # Check that the warning was called for the unknown key mock_warning.assert_called_once() args, _ = mock_warning.call_args - assert isinstance(args[0], ConfigError) \ No newline at end of file + assert isinstance(args[0], ConfigError) From e0d76ec8b547105661bcdada1b66fd232ccefedc Mon Sep 17 00:00:00 2001 From: vschutze-alt Date: Mon, 26 May 2025 09:47:11 +0100 Subject: [PATCH 4/6] add test that correct keys are present in closure_test_model_settings --- colibri/config.py | 25 +++++++++++++++++++++---- colibri/tests/test_config.py | 1 - 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/colibri/config.py b/colibri/config.py index 827ddaeb9..f84b1dd45 100644 --- a/colibri/config.py +++ b/colibri/config.py @@ -523,6 +523,10 @@ def produce_pdf_model(self): return None def parse_closure_test_colibri_model_pdf(self, settings): + """ + 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"} kdiff = settings.keys() - known_keys @@ -536,11 +540,24 @@ def parse_closure_test_colibri_model_pdf(self, settings): ) # Now construct the closure_test_model_settings dictionary, - # checking the parameter combinations are valid + # checking the required keys are present. closure_test_model_settings = {} - closure_test_model_settings["model"] = settings.get("model") - closure_test_model_settings["fitted_flavours"] = settings.get("fitted_flavours") - closure_test_model_settings["parameters"] = settings.get("parameters") + if "model" in settings: + closure_test_model_settings["model"] = settings.get("model") + else: + raise KeyError(f"model not found in closure_test_model_settings") + + if "fitted_flavours" in settings: + closure_test_model_settings["fitted_flavours"] = settings.get( + "fitted_flavours" + ) + else: + raise KeyError(f"fitted_flavours not found in closure_test_model_settings") + + if "parameters" in settings: + closure_test_model_settings["parameters"] = settings.get("parameters") + else: + raise KeyError(f"parameters not found in closure_test_model_settings") return closure_test_model_settings diff --git a/colibri/tests/test_config.py b/colibri/tests/test_config.py index 39a05078e..70724e947 100644 --- a/colibri/tests/test_config.py +++ b/colibri/tests/test_config.py @@ -423,7 +423,6 @@ def test_produce_commondata_tuple(): @patch("colibri.config.log.warning") def test_parse_closure_test_colibri_model_pdf(mock_warning): # Mock the pdf model - settings = { "parameters": {"param1": 1, "param2": 2}, "model": "test_model", From 71407ea068a8267844484f98d51ce52a958f8e46 Mon Sep 17 00:00:00 2001 From: vschutze-alt Date: Mon, 26 May 2025 09:58:00 +0100 Subject: [PATCH 5/6] update test_config to cover all code in parse_closure_test_colibri_model_pdf --- colibri/tests/test_config.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/colibri/tests/test_config.py b/colibri/tests/test_config.py index 70724e947..462673217 100644 --- a/colibri/tests/test_config.py +++ b/colibri/tests/test_config.py @@ -446,3 +446,29 @@ def test_parse_closure_test_colibri_model_pdf(mock_warning): mock_warning.assert_called_once() args, _ = mock_warning.call_args assert isinstance(args[0], ConfigError) + + # Check that error is raised if a necessary key is missing + + settings_no_model = { + "parameters": {"param1": 1}, + "fitted_flavours": ["T3"], + } + + with pytest.raises(KeyError, match="model not found"): + BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings_no_model) + + settings_no_fitted_flavours = { + "parameters": {"param1": 1}, + "model": "test_model", + } + + with pytest.raises(KeyError, match="fitted_flavours not found"): + BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings_no_fitted_flavours) + + settings_no_parameters = { + "model": "test_model", + "fitted_flavours": ["T3"], + } + + with pytest.raises(KeyError, match="parameters not found"): + BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings_no_parameters) From c507071ce30403531139faa4ded0dcc95a7b27f1 Mon Sep 17 00:00:00 2001 From: vschutze-alt Date: Tue, 3 Jun 2025 16:13:09 +0100 Subject: [PATCH 6/6] remove fitted flavours as a requirement in parse_closure_test_colibri_model_pdf and allow for any keys, as long as parameters and model are there. Adapt unit test accordingly --- colibri/config.py | 43 +++++++++--------------------------- colibri/tests/test_config.py | 20 +++-------------- 2 files changed, 13 insertions(+), 50 deletions(-) diff --git a/colibri/config.py b/colibri/config.py index f84b1dd45..dc3bf29e4 100644 --- a/colibri/config.py +++ b/colibri/config.py @@ -524,40 +524,17 @@ def produce_pdf_model(self): def parse_closure_test_colibri_model_pdf(self, settings): """ - Checks that the required keys are included in closure_test_model_settings - and constructs a dictionary for the settings. + Validates that required keys are present and returns the full settings dictionary. + Requires: 'model' and 'parameters'. + Other keys (e.g. 'fitted_flavours') are allowed and passed through. """ - known_keys = {"model", "fitted_flavours", "parameters"} + required_keys = {"model", "parameters"} - kdiff = settings.keys() - known_keys - for k in kdiff: - log.warning( - ConfigError( - f"Key '{k}' in closure_test_model_settings not known.", - k, - known_keys, - ) + missing_keys = required_keys - settings.keys() + if missing_keys: + raise KeyError( + f"Missing required key(s) in closure_test_model_settings: {', '.join(missing_keys)}" ) - # Now construct the closure_test_model_settings dictionary, - # checking the required keys are present. - closure_test_model_settings = {} - - if "model" in settings: - closure_test_model_settings["model"] = settings.get("model") - else: - raise KeyError(f"model not found in closure_test_model_settings") - - if "fitted_flavours" in settings: - closure_test_model_settings["fitted_flavours"] = settings.get( - "fitted_flavours" - ) - else: - raise KeyError(f"fitted_flavours not found in closure_test_model_settings") - - if "parameters" in settings: - closure_test_model_settings["parameters"] = settings.get("parameters") - else: - raise KeyError(f"parameters not found in closure_test_model_settings") - - return closure_test_model_settings + # Return a full copy of the settings dictionary (assuming it’s valid) + return dict(settings) diff --git a/colibri/tests/test_config.py b/colibri/tests/test_config.py index 462673217..9f415141b 100644 --- a/colibri/tests/test_config.py +++ b/colibri/tests/test_config.py @@ -427,7 +427,6 @@ def test_parse_closure_test_colibri_model_pdf(mock_warning): "parameters": {"param1": 1, "param2": 2}, "model": "test_model", "fitted_flavours": ["T3", "T8"], - "unknown_key": "should_warn", } # Call the method @@ -435,18 +434,13 @@ def test_parse_closure_test_colibri_model_pdf(mock_warning): # Assert the result is as expected expected = { + "parameters": {"param1": 1, "param2": 2}, "model": "test_model", "fitted_flavours": ["T3", "T8"], - "parameters": {"param1": 1, "param2": 2}, } assert result == expected - # Check that the warning was called for the unknown key - mock_warning.assert_called_once() - args, _ = mock_warning.call_args - assert isinstance(args[0], ConfigError) - # Check that error is raised if a necessary key is missing settings_no_model = { @@ -454,21 +448,13 @@ def test_parse_closure_test_colibri_model_pdf(mock_warning): "fitted_flavours": ["T3"], } - with pytest.raises(KeyError, match="model not found"): + with pytest.raises(KeyError, match="Missing required key.*model"): BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings_no_model) - settings_no_fitted_flavours = { - "parameters": {"param1": 1}, - "model": "test_model", - } - - with pytest.raises(KeyError, match="fitted_flavours not found"): - BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings_no_fitted_flavours) - settings_no_parameters = { "model": "test_model", "fitted_flavours": ["T3"], } - with pytest.raises(KeyError, match="parameters not found"): + with pytest.raises(KeyError, match="Missing required key.*parameters"): BASE_CONFIG.parse_closure_test_colibri_model_pdf(settings_no_parameters)