From f499e57f74764097784b14c4830ca73dcaaebe60 Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 18:25:19 +0100 Subject: [PATCH 1/8] test: extract test_estimator --- tests/conftest.py | 6 ++++++ tests/optimizer/test_minuit.py | 20 +------------------- tests/test_estimator.py | 22 ++++++++++++++++++++++ 3 files changed, 29 insertions(+), 19 deletions(-) create mode 100644 tests/test_estimator.py diff --git a/tests/conftest.py b/tests/conftest.py index 2717b8b4..061a8ed4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,7 @@ from tensorwaves.data.generate import generate_data, generate_phsp from tensorwaves.data.tf_phasespace import TFUniformRealNumberGenerator +from tensorwaves.estimator import UnbinnedNLL from tensorwaves.physics.helicity_formalism.amplitude import ( IntensityBuilder, IntensityTF, @@ -80,6 +81,11 @@ def data_set( return kinematics.convert(data_sample) +@pytest.fixture(scope="session") +def estimator(intensity: IntensityTF, data_set: dict) -> UnbinnedNLL: + return UnbinnedNLL(intensity, data_set) + + def __create_model(formalism: str) -> AmplitudeModel: result = es.generate_transitions( initial_state=("J/psi(1S)", [-1, +1]), diff --git a/tests/optimizer/test_minuit.py b/tests/optimizer/test_minuit.py index f1f05e58..f198cd6d 100644 --- a/tests/optimizer/test_minuit.py +++ b/tests/optimizer/test_minuit.py @@ -4,29 +4,11 @@ from tensorwaves.estimator import UnbinnedNLL from tensorwaves.optimizer.minuit import Minuit2 -from tensorwaves.physics.helicity_formalism.amplitude import IntensityTF class TestMinuit2: @staticmethod - def test_optimize(intensity: IntensityTF, data_set: dict): - estimator = UnbinnedNLL(intensity, data_set) - assert estimator.parameters == { - "strength_incoherent": 1.0, - "MesonRadius_J/psi(1S)": 1.0, - "MesonRadius_f(0)(500)": 1.0, - "MesonRadius_f(0)(980)": 1.0, - "Magnitude_J/psi(1S)_to_f(0)(500)_0+gamma_1;f(0)(500)_to_pi0_0+pi0_0;": 1.0, - "Phase_J/psi(1S)_to_f(0)(500)_0+gamma_1;f(0)(500)_to_pi0_0+pi0_0;": 0.0, - "Magnitude_J/psi(1S)_to_f(0)(980)_0+gamma_1;f(0)(980)_to_pi0_0+pi0_0;": 1.0, - "Phase_J/psi(1S)_to_f(0)(980)_0+gamma_1;f(0)(980)_to_pi0_0+pi0_0;": 0.0, - "Mass_J/psi(1S)": 3.0969, - "Width_J/psi(1S)": 9.29e-05, - "Mass_f(0)(500)": 0.475, - "Width_f(0)(500)": 0.55, - "Mass_f(0)(980)": 0.99, - "Width_f(0)(980)": 0.06, - } + def test_optimize(estimator: UnbinnedNLL): free_pars = { "Width_f(0)(500)": 0.3, "Mass_f(0)(980)": 1, diff --git a/tests/test_estimator.py b/tests/test_estimator.py new file mode 100644 index 00000000..bc703af2 --- /dev/null +++ b/tests/test_estimator.py @@ -0,0 +1,22 @@ +from tensorwaves.estimator import UnbinnedNLL + + +class TestUnbinnedNLL: + @staticmethod + def test_parameters(estimator: UnbinnedNLL): + assert estimator.parameters == { + "strength_incoherent": 1.0, + "MesonRadius_J/psi(1S)": 1.0, + "MesonRadius_f(0)(500)": 1.0, + "MesonRadius_f(0)(980)": 1.0, + "Magnitude_J/psi(1S)_to_f(0)(500)_0+gamma_1;f(0)(500)_to_pi0_0+pi0_0;": 1.0, + "Phase_J/psi(1S)_to_f(0)(500)_0+gamma_1;f(0)(500)_to_pi0_0+pi0_0;": 0.0, + "Magnitude_J/psi(1S)_to_f(0)(980)_0+gamma_1;f(0)(980)_to_pi0_0+pi0_0;": 1.0, + "Phase_J/psi(1S)_to_f(0)(980)_0+gamma_1;f(0)(980)_to_pi0_0+pi0_0;": 0.0, + "Mass_J/psi(1S)": 3.0969, + "Width_J/psi(1S)": 9.29e-05, + "Mass_f(0)(500)": 0.475, + "Width_f(0)(500)": 0.55, + "Mass_f(0)(980)": 0.99, + "Width_f(0)(980)": 0.06, + } From 6f00f18367b5f70afddf0ae435fe0941a318cd69 Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 18:57:24 +0100 Subject: [PATCH 2/8] fix: copy initial parameters in optimize call --- src/tensorwaves/optimizer/minuit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tensorwaves/optimizer/minuit.py b/src/tensorwaves/optimizer/minuit.py index ae90560e..e640b1e4 100644 --- a/src/tensorwaves/optimizer/minuit.py +++ b/src/tensorwaves/optimizer/minuit.py @@ -1,6 +1,7 @@ """Minuit2 adapter to the `iminuit.Minuit` package.""" import time +from copy import deepcopy from typing import Optional from iminuit import Minuit @@ -22,7 +23,7 @@ def __init__(self, callback: Optional[Callback] = None) -> None: self.__callback = callback def optimize(self, estimator: Estimator, initial_parameters: dict) -> dict: - parameters = initial_parameters + parameters = deepcopy(initial_parameters) def __wrapped_function(pars: list) -> float: for i, k in enumerate(parameters.keys()): From 340c5816d29418d9bd2fdd17650915a9997306cc Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 18:57:52 +0100 Subject: [PATCH 3/8] fix: type hint Minuit2.optimize --- src/tensorwaves/optimizer/minuit.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tensorwaves/optimizer/minuit.py b/src/tensorwaves/optimizer/minuit.py index e640b1e4..be76961b 100644 --- a/src/tensorwaves/optimizer/minuit.py +++ b/src/tensorwaves/optimizer/minuit.py @@ -2,7 +2,7 @@ import time from copy import deepcopy -from typing import Optional +from typing import Dict, Optional from iminuit import Minuit @@ -22,7 +22,9 @@ def __init__(self, callback: Optional[Callback] = None) -> None: if callback is not None: self.__callback = callback - def optimize(self, estimator: Estimator, initial_parameters: dict) -> dict: + def optimize( + self, estimator: Estimator, initial_parameters: Dict[str, float] + ) -> dict: parameters = deepcopy(initial_parameters) def __wrapped_function(pars: list) -> float: From 171da52d78aed946fee2da465d9b85190db4ed70 Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 18:58:55 +0100 Subject: [PATCH 4/8] refactor: change fit result dict structure --- src/tensorwaves/optimizer/minuit.py | 24 +++++++++++++----------- tests/optimizer/test_minuit.py | 16 +++++++++------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/tensorwaves/optimizer/minuit.py b/src/tensorwaves/optimizer/minuit.py index be76961b..a856a6db 100644 --- a/src/tensorwaves/optimizer/minuit.py +++ b/src/tensorwaves/optimizer/minuit.py @@ -52,15 +52,17 @@ def __wrapped_function(pars: list) -> float: par_states = minuit.params f_min = minuit.fmin - results: dict = {"params": {}} + parameter_values = dict() + parameter_errors = dict() for i, name in enumerate(parameters.keys()): - results["params"][name] = ( - par_states[i].value, - par_states[i].error, - ) - - # return fit results - results["log_lh"] = f_min.fval - results["func_calls"] = f_min.ncalls - results["time"] = end_time - start_time - return results + par_state = par_states[i] + parameter_values[name] = par_state.value + parameter_errors[name] = par_state.error + + return { + "parameter_values": parameter_values, + "parameter_errors": parameter_errors, + "log_likelihood": f_min.fval, + "function_calls": f_min.ncalls, + "execution_time": end_time - start_time, + } diff --git a/tests/optimizer/test_minuit.py b/tests/optimizer/test_minuit.py index f198cd6d..be6aa0cf 100644 --- a/tests/optimizer/test_minuit.py +++ b/tests/optimizer/test_minuit.py @@ -16,12 +16,14 @@ def test_optimize(estimator: UnbinnedNLL): optimizer = Minuit2() result = optimizer.optimize(estimator, free_pars) assert set(result) == { - "params", - "log_lh", - "func_calls", + "parameter_values", + "parameter_errors", + "log_likelihood", + "function_calls", "time", } - assert set(result["params"]) == set(free_pars) - assert pytest.approx(result["log_lh"]) == -13379.223862030514 - assert pytest.approx(free_pars["Width_f(0)(500)"]) == 0.559522579972911 - assert pytest.approx(free_pars["Mass_f(0)(980)"]) == 0.9901984320598398 + par_values = result["parameter_values"] + assert set(par_values) == set(free_pars) + assert pytest.approx(result["log_likelihood"]) == -13379.223862030514 + assert pytest.approx(par_values["Width_f(0)(500)"]) == 0.55868526502471 + assert pytest.approx(par_values["Mass_f(0)(980)"]) == 0.990141023090767 From 655461b245e2119abebddaa7a22dbd561add75fb Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 18:59:44 +0100 Subject: [PATCH 5/8] test: parameter errors --- tests/optimizer/test_minuit.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/optimizer/test_minuit.py b/tests/optimizer/test_minuit.py index be6aa0cf..05703d2f 100644 --- a/tests/optimizer/test_minuit.py +++ b/tests/optimizer/test_minuit.py @@ -23,7 +23,10 @@ def test_optimize(estimator: UnbinnedNLL): "time", } par_values = result["parameter_values"] + par_errors = result["parameter_errors"] assert set(par_values) == set(free_pars) assert pytest.approx(result["log_likelihood"]) == -13379.223862030514 assert pytest.approx(par_values["Width_f(0)(500)"]) == 0.55868526502471 + assert pytest.approx(par_errors["Width_f(0)(500)"]) == 0.01057804923356 assert pytest.approx(par_values["Mass_f(0)(980)"]) == 0.990141023090767 + assert pytest.approx(par_errors["Mass_f(0)(980)"]) == 0.000721352674347 From 0e3a13def45296bea16a4ba8490b4ce456d41a54 Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 19:00:14 +0100 Subject: [PATCH 6/8] docs: get optimized parameters from result --- docs/usage/3_perform_fit.ipynb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/usage/3_perform_fit.ipynb b/docs/usage/3_perform_fit.ipynb index 98f4a217..d0504bf6 100644 --- a/docs/usage/3_perform_fit.ipynb +++ b/docs/usage/3_perform_fit.ipynb @@ -256,6 +256,23 @@ "```" ] }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "As can be seen, the values of the optimized parameters in the result are again comparable to the original values we saw in {ref}`usage/3_perform_fit:3.1 Define estimator`:" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "optimized_parameters = result[\"parameter_values\"]\n", + "optimized_parameters" + ] + }, { "cell_type": "markdown", "metadata": {}, From 4fd0218471b21be5064be11a8d955de9aefde28e Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 19:05:36 +0100 Subject: [PATCH 7/8] fix: execution_time key --- tests/optimizer/test_minuit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/optimizer/test_minuit.py b/tests/optimizer/test_minuit.py index 05703d2f..dccbddd3 100644 --- a/tests/optimizer/test_minuit.py +++ b/tests/optimizer/test_minuit.py @@ -20,7 +20,7 @@ def test_optimize(estimator: UnbinnedNLL): "parameter_errors", "log_likelihood", "function_calls", - "time", + "execution_time", } par_values = result["parameter_values"] par_errors = result["parameter_errors"] From 6832aed7bf2de303f5871016537ab04685a91ffd Mon Sep 17 00:00:00 2001 From: Remco de Boer Date: Fri, 20 Nov 2020 19:21:25 +0100 Subject: [PATCH 8/8] fix: temporary fix for #171 --- tests/conftest.py | 5 ++++- tests/recipe/test_amplitude_creation.py | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 061a8ed4..6f9db805 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,7 @@ # pylint: disable=redefined-outer-name +from copy import deepcopy + import expertsystem as es import numpy as np import pytest @@ -58,7 +60,8 @@ def intensity( kinematics: HelicityKinematics, phsp_sample: np.ndarray, ) -> IntensityTF: - model = helicity_model + # https://github.com/ComPWA/tensorwaves/issues/171 + model = deepcopy(helicity_model) builder = IntensityBuilder(model.particles, kinematics, phsp_sample) return builder.create_intensity(model) diff --git a/tests/recipe/test_amplitude_creation.py b/tests/recipe/test_amplitude_creation.py index c93393ff..b9383f22 100644 --- a/tests/recipe/test_amplitude_creation.py +++ b/tests/recipe/test_amplitude_creation.py @@ -1,4 +1,5 @@ import os +from copy import deepcopy import expertsystem.amplitude.model as es @@ -21,7 +22,8 @@ def _generate_phsp(recipe: es.AmplitudeModel, number_of_events: int): def test_helicity(helicity_model: es.AmplitudeModel): - model = helicity_model + # https://github.com/ComPWA/tensorwaves/issues/171 + model = deepcopy(helicity_model) kinematics = HelicityKinematics.from_model(model) masses_is = kinematics.reaction_kinematics_info.initial_state_masses masses_fs = kinematics.reaction_kinematics_info.final_state_masses @@ -37,7 +39,8 @@ def test_helicity(helicity_model: es.AmplitudeModel): def test_canonical(canonical_model: es.AmplitudeModel): - model = canonical_model + # https://github.com/ComPWA/tensorwaves/issues/171 + model = deepcopy(canonical_model) particles = model.particles kinematics = HelicityKinematics.from_model(model) phsp_sample = _generate_phsp(model, NUMBER_OF_PHSP_EVENTS)