diff --git a/gammapy/scripts/tests/test_spectrum_pipe.py b/gammapy/scripts/tests/test_spectrum_pipe.py index 636636051d5..8228d984b90 100644 --- a/gammapy/scripts/tests/test_spectrum_pipe.py +++ b/gammapy/scripts/tests/test_spectrum_pipe.py @@ -16,6 +16,9 @@ def get_config(): amplitude=1e-11 * u.Unit('cm-2 s-1 TeV-1'), reference=1 * u.TeV, ) + model.parameters.set_parameter_errors({ + 'amplitude' : 1e-12 * u.Unit('cm-2 s-1 TeV-1') + }) fp_binning = EnergyBounds.equal_log_spacing(1, 50, 4, 'TeV') return dict( outdir=None, diff --git a/gammapy/spectrum/fit.py b/gammapy/spectrum/fit.py index 72465635593..944298e6678 100644 --- a/gammapy/spectrum/fit.py +++ b/gammapy/spectrum/fit.py @@ -41,9 +41,7 @@ class SpectrumFit(object): Fit range, will be convolved with observation thresholds. If you want to control which bins are taken into account in the fit for each observation, use :func:`~gammapy.spectrum.PHACountsSpectrum.quality` - background_model : `~gammapy.spectrum.models.SpectralModel`, optional - Background model to be used in cash fits - method : {'sherpa', 'iminuit'} + method : {'iminuit'} Optimization backend for the fit error_method : {'covar', 'conf', 'HESSE', 'MINOS'} Method of the error estimation depending on the backend. @@ -51,14 +49,12 @@ class SpectrumFit(object): """ def __init__(self, obs_list, model, stat='wstat', forward_folded=True, - fit_range=None, background_model=None, - method='sherpa', error_method=None): + fit_range=None, method='iminuit', error_method=None): self.obs_list = obs_list self._model = model self.stat = stat self.forward_folded = forward_folded self.fit_range = fit_range - self._background_model = background_model self.method = method self.error_method = error_method @@ -78,8 +74,6 @@ def __str__(self): ss += '\nStat {}'.format(self.stat) ss += '\nForward Folded {}'.format(self.forward_folded) ss += '\nFit range {}'.format(self.fit_range) - if self.background_model is not None: - ss += '\nBackground model {}'.format(self.background_model) ss += '\nBackend {}'.format(self.method) ss += '\nError Method {}'.format(self.error_method) @@ -97,20 +91,6 @@ def result(self): """ return self._result - @property - def background_model(self): - """Background model - - The model parameters change every time the likelihood is evaluated. In - order to access the best-fit model parameters, use - :func:`gammapy.spectrum.SpectrumFit.result` - """ - return self._background_model - - @background_model.setter - def background_model(self, model): - self._background_model = model - @property def obs_list(self): """Observations participating in the fit""" @@ -214,14 +194,7 @@ def predict_counts(self): mu_sig = self._predict_counts_helper(obs, self._model, self.forward_folded) - mu_bkg = None - if self.background_model is not None: - # For now, never fold background model with IRFs - mu_bkg = self._predict_counts_helper(obs, - self.background_model, - False) - counts = [mu_sig, mu_bkg] - predicted_counts.append(counts) + predicted_counts.append(mu_sig) self._predicted_counts = predicted_counts def _predict_counts_helper(self, obs, model, forward_folded=True): @@ -272,9 +245,8 @@ def calc_statval(self): """ statval = [] for obs, npred in zip(self.obs_list, self.predicted_counts): - on_stat, off_stat = self._calc_statval_helper(obs, npred) - statvals = (on_stat, off_stat) - statval.append(statvals) + on_stat = self._calc_statval_helper(obs, npred) + statval.append(on_stat) self._statval = statval self._restrict_statval() @@ -286,48 +258,30 @@ def _calc_statval_helper(self, obs, prediction): obs : `~gammapy.spectrum.SpectrumObservation` Measured counts prediction : tuple of `~numpy.ndarray` - Predicted (on counts, off counts) + Predicted counts Returns ------ statsval : tuple of `~numpy.ndarray` - Statval for (on, off) + Statval """ stats_func = getattr(stats, self.stat) - # Off stat = 0 by default - off_stat = np.zeros(obs.e_reco.nbins) if self.stat == 'cash' or self.stat == 'cstat': - if self.background_model is not None: - mu_on = prediction[0] + prediction[1] - on_stat = stats_func(n_on=obs.on_vector.data.data.value, - mu_on=mu_on) - mu_off = prediction[1] / obs.alpha - off_stat = stats_func(n_on=obs.off_vector.data.data.value, - mu_on=mu_off) - else: - mu_on = prediction[0] - on_stat = stats_func(n_on=obs.on_vector.data.data.value, - mu_on=mu_on) - off_stat = np.zeros_like(on_stat) + on_stat = stats_func(n_on=obs.on_vector.data.data.value, + mu_on=prediction) elif self.stat == 'wstat': kwargs = dict(n_on=obs.on_vector.data.data.value, n_off=obs.off_vector.data.data.value, alpha=obs.alpha, - mu_sig=prediction[0]) - # Store the result of the profile likelihood as bkg prediction - mu_bkg = stats.get_wstat_mu_bkg(**kwargs) - prediction[1] = mu_bkg * obs.alpha + mu_sig=prediction) on_stat_ = stats_func(**kwargs) - # The on_stat sometime contains nan values - # TODO: Handle properly on_stat = np.nan_to_num(on_stat_) - off_stat = np.zeros_like(on_stat) else: raise NotImplementedError('{}'.format(self.stat)) - return on_stat, off_stat + return on_stat def total_stat(self, parameters): """Statistic summed over all bins and all observations. @@ -351,8 +305,7 @@ def _restrict_statval(self): for statval, valid_range in zip(self.statval, self.bins_in_fit_range): # Find bins outside safe range idx = np.where(np.invert(valid_range))[0] - statval[0][idx] = 0 - statval[1][idx] = 0 + statval[idx] = 0 def _check_valid_fit(self): """Helper function to give useful error messages.""" @@ -410,48 +363,11 @@ def fit(self, opts_minuit=None): opts_minuit : dict (optional) Options passed to `iminuit.Minuit` constructor """ - if self.method == 'sherpa': - self._fit_sherpa() - elif self.method == 'iminuit': + if self.method == 'iminuit': self._fit_iminuit(opts_minuit) else: raise NotImplementedError('method: {}'.format(self.method)) - def _fit_sherpa(self): - """Wrapper around sherpa minimizer.""" - from sherpa.fit import Fit - from sherpa.data import Data1DInt - from sherpa.optmethods import NelderMead - from .sherpa_utils import SherpaModel, SherpaStat - - binning = self.obs_list[0].e_reco - # The sherpa data object is not usued in the fit. It is set to the - # first observation for debugging purposes, see below - data = self.obs_list[0].on_vector.data.data.value - data = Data1DInt('Dummy data', binning[:-1].value, - binning[1:].value, data) - # DEBUG - # from sherpa.models import PowLaw1D - # from sherpa.stats import Cash - # model = PowLaw1D('sherpa') - # model.ref = 0.1 - # fit = Fit(data, model, Cash(), NelderMead()) - - # NOTE: We cannot use the Levenbergr-Marquart optimizer in Sherpa - # because it relies on the fvec return value of the fit statistic (we - # return None). The computation of fvec is not straightforwad, not just - # stats per bin. E.g. for a cash fit the sherpa stat computes it - # according to cstat - # see https://github.com/sherpa/sherpa/blob/master/sherpa/include/sherpa/stats.hh#L122 - - self._sherpa_fit = Fit(data, - SherpaModel(self), - SherpaStat(self), - NelderMead()) - fitresult = self._sherpa_fit.fit() - log.debug(fitresult) - self._make_fit_result(self._model.parameters) - def _fit_iminuit(self, opts_minuit): """Iminuit minimization""" parameters, minuit = fit_iminuit(parameters=self._model.parameters, @@ -475,11 +391,6 @@ def _make_fit_result(self, parameters): self.total_stat(parameters) model = self._model.copy() - if self.background_model is not None: - bkg_model = self.background_model.copy() - else: - bkg_model = None - statname = self.stat results = [] @@ -487,8 +398,7 @@ def _make_fit_result(self, parameters): fit_range = self.true_fit_range[idx] statval = np.sum(self.statval[idx]) stat_per_bin = self.statval[idx] - npred_src = copy.deepcopy(self.predicted_counts[idx][0]) - npred_bkg = copy.deepcopy(self.predicted_counts[idx][1]) + npred_src = copy.deepcopy(self.predicted_counts[idx]) results.append(SpectrumFitResult( model=model, @@ -497,8 +407,6 @@ def _make_fit_result(self, parameters): statval=statval, stat_per_bin=stat_per_bin, npred_src=npred_src, - npred_bkg=npred_bkg, - background_model=bkg_model, obs=obs )) @@ -506,9 +414,7 @@ def _make_fit_result(self, parameters): def est_errors(self): """Estimate parameter errors.""" - if self.method == 'sherpa': - self._est_errors_sherpa() - elif self.method == 'iminuit': + if self.method == 'iminuit': self._est_errors_iminuit() else: raise NotImplementedError('{}'.format(self.method)) @@ -538,12 +444,6 @@ def _est_errors_iminuit(self): cov = cov.reshape(len(parameter_names), -1) self.covariance = cov - def _est_errors_sherpa(self): - """Wrapper around Sherpa error estimator.""" - covar = self._sherpa_fit.est_errors() - self.covar_axis = [par.split('.')[-1] for par in covar.parnames] - self.covariance = copy.deepcopy(covar.extra_output) - def run(self, outdir=None): """Run all steps and write result to disk. diff --git a/gammapy/spectrum/models.py b/gammapy/spectrum/models.py index 5accb87ca21..e217334423b 100644 --- a/gammapy/spectrum/models.py +++ b/gammapy/spectrum/models.py @@ -879,7 +879,33 @@ def to_sherpa(self, name='default'): name : str, optional Name of the sherpa model instance """ - from .sherpa_utils import SherpaExponentialCutoffPowerLaw + from sherpa.models import ArithmeticModel, modelCacher1d, Parameter + + class SherpaExponentialCutoffPowerLaw(ArithmeticModel): + def __init__(self, name='ecpl'): + self.gamma = Parameter(name, 'gamma', 2, min=-10, max=10) + self.ref = Parameter(name, 'ref', 1, frozen=True) + self.ampl = Parameter(name, 'ampl', 1, min=0) + self.cutoff = Parameter(name, 'cutoff', 1, min=0, units='1/TeV') + ArithmeticModel.__init__(self, name, (self.gamma, self.ref, + self.ampl, self.cutoff)) + self._use_caching = True + self.cache = 10 + + @modelCacher1d + def calc(self, p, x, xhi=None): + kev_to_tev = 1e-9 + model = ExponentialCutoffPowerLaw(index=p[0], + reference=p[1], + amplitude=p[2], + lambda_=p[3] * kev_to_tev) + if xhi is None: + val = model(x) + else: + val = model.integral(x, xhi, intervals=True) + + return val + model = SherpaExponentialCutoffPowerLaw(name='ecpl.' + name) pars = self.parameters diff --git a/gammapy/spectrum/sherpa_utils.py b/gammapy/spectrum/sherpa_utils.py deleted file mode 100644 index e7c513c5292..00000000000 --- a/gammapy/spectrum/sherpa_utils.py +++ /dev/null @@ -1,115 +0,0 @@ -# Licensed under a 3-clause BSD style license - see LICENSE.rst -"""This module contains helper classes for the sherpa backend of -`~gammapy.spectrum.SpectrumFit. -""" -from __future__ import absolute_import, division, print_function, unicode_literals -import numpy as np -from sherpa.models import ArithmeticModel, modelCacher1d, Parameter -from sherpa.stats import Likelihood - -__all__ = [ - 'SherpaModel', - 'SherpaStat', - 'SherpaExponentialCutoffPowerLaw', -] - - -class SherpaModel(ArithmeticModel): - """Dummy sherpa model for the `~gammapy.spectrum.SpectrumFit` - - Parameters - ---------- - fit : `~gammapy.spectrum.SpectrumFit` - Fit instance - """ - - def __init__(self, fit): - self.fit = fit - - sherpa_name = 'sherpa_model' - par_list = list() - for par in self.fit._model.parameters.parameters: - sherpa_par = par.to_sherpa(modelname='source') - # setattr(self, name, sherpa_par) - par_list.append(sherpa_par) - - if fit.stat != 'wstat' and self.fit.background_model is not None: - for par in self.fit.background_model.parameters.parameters: - sherpa_par = par.to_sherpa(modelname='background') - # setattr(self, name, sherpa_par) - par_list.append(sherpa_par) - - ArithmeticModel.__init__(self, sherpa_name, par_list) - self._use_caching = True - self.cache = 10 - - @modelCacher1d - def calc(self, p, x, xhi=None): - # Adjust model parameters - n_src = len(self.fit._model.parameters.parameters) - for idx, par in enumerate(p): - # Special case background model - if idx >= n_src: - self.fit.background_model.parameters.parameters[idx - n_src].value = par - else: - self.fit._model.parameters.parameters[idx].value = par - - self.fit.predict_counts() - # Return ones since sherpa does some check on the shape - return np.ones_like(self.fit.obs_list[0].e_reco) - - -class SherpaStat(Likelihood): - """Dummy sherpa stat for the `~gammapy.spectrum.SpectrumFit` - - Parameters - ---------- - fit : `~gammapy.spectrum.SpectrumFit` - Fit instance - """ - - def __init__(self, fit): - sherpa_name = 'sherpa_stat' - self.fit = fit - Likelihood.__init__(self, sherpa_name) - - def _calc(self, data, model, *args, **kwargs): - self.fit.calc_statval() - # Sum likelihood over all observations - total_stat = np.sum([np.sum(v) for v in self.fit.statval], dtype=np.float64) - # sherpa return pattern: total stat, fvec - return total_stat, None - - -class SherpaExponentialCutoffPowerLaw(ArithmeticModel): - """Exponential CutoffPowerLaw. - - Note that the cutoff is given in units '1/TeV' in order to bring the Sherpa - optimizers into a valid range. All other parameters still have units 'keV' - and 'cm2'. - """ - - def __init__(self, name='ecpl'): - self.gamma = Parameter(name, 'gamma', 2, min=-10, max=10) - self.ref = Parameter(name, 'ref', 1, frozen=True) - self.ampl = Parameter(name, 'ampl', 1, min=0) - self.cutoff = Parameter(name, 'cutoff', 1, min=0, units='1/TeV') - ArithmeticModel.__init__(self, name, (self.gamma, self.ref, self.ampl, - self.cutoff)) - self._use_caching = True - self.cache = 10 - - @modelCacher1d - def calc(self, p, x, xhi=None): - from .models import ExponentialCutoffPowerLaw - kev_to_tev = 1e-9 - model = ExponentialCutoffPowerLaw(index=p[0], - reference=p[1], - amplitude=p[2], - lambda_=p[3] * kev_to_tev) - if xhi is None: - val = model(x) - else: - val = model.integral(x, xhi, intervals=True) - - return val diff --git a/gammapy/spectrum/tests/test_fit.py b/gammapy/spectrum/tests/test_fit.py index d5a5d79f4f2..6fae49dca73 100644 --- a/gammapy/spectrum/tests/test_fit.py +++ b/gammapy/spectrum/tests/test_fit.py @@ -65,7 +65,7 @@ def test_cash(self): assert 'Spectrum' in str(fit) fit.predict_counts() - assert_allclose(fit.predicted_counts[0][0][5], 660.5171280778071) + assert_allclose(fit.predicted_counts[0][5], 660.5171280778071) fit.calc_statval() assert_allclose(np.sum(fit.statval[0]), -107346.5291329714) @@ -74,29 +74,10 @@ def test_cash(self): fit.fit() # These values are check with sherpa fits, do not change assert_allclose(fit.result[0].model.parameters['index'].value, - 1.9955563477414806) + 1.996753564321903) assert_allclose(fit.result[0].model.parameters['amplitude'].value, - 100250.33102108649) + 100404.1118915871) - def test_cash_with_bkg(self): - """Cash fit taking into account background model""" - on_vector = self.src.copy() - on_vector.data.data += self.bkg.data.data - obs = SpectrumObservation(on_vector=on_vector, off_vector=self.off) - obs_list = SpectrumObservationList([obs]) - - self.source_model.parameters['index'].value = 1 - self.bkg_model.parameters['index'].value = 1 - fit = SpectrumFit(obs_list=obs_list, model=self.source_model, - stat='cash', forward_folded=False, - background_model=self.bkg_model) - assert 'Background' in str(fit) - - fit.fit() - assert_allclose(fit.result[0].model.parameters['index'].value, - 1.996272386763962) - assert_allclose(fit.result[0].background_model.parameters['index'].value, - 2.9926225268193418) def test_wstat(self): """WStat with on source and background spectrum""" @@ -110,10 +91,10 @@ def test_wstat(self): stat='wstat', forward_folded=False) fit.fit() assert_allclose(fit.result[0].model.parameters['index'].value, - 1.997344538577775) + 1.9973633924241248) assert_allclose(fit.result[0].model.parameters['amplitude'].value, - 100244.89943081759) - assert_allclose(fit.result[0].statval, 30.022315611837342) + 100247.95103265066) + assert_allclose(fit.result[0].statval, 30.022319652565265) def test_joint(self): @@ -125,7 +106,7 @@ def test_joint(self): model=self.source_model, forward_folded=False) fit.fit() assert_allclose(fit.result[0].model.parameters['index'].value, - 1.9964768894266016) + 1.9977254068253105) def test_fit_range(self): """Test fit range without complication of thresholds""" @@ -205,23 +186,8 @@ def setup(self): # Example fit for one observation self.fit = SpectrumFit(self.obs_list[0], self.pwl) - def test_basic_results(self): - self.fit.fit() - result = self.fit.result[0] - assert self.fit.method == 'sherpa' - assert_allclose(result.statval, 32.838716584005645) - pars = result.model.parameters - assert_quantity_allclose(pars['index'].value, 2.2542312883476465) - assert_quantity_allclose(pars['amplitude'].quantity, - 2.0082864582748925e-7 * u.Unit('m-2 s-1 TeV-1')) - assert_allclose(result.npred_src[60], 0.563822994375907) - - # TODO: at the moment to_table only works if covariance matrix is set - with pytest.raises(ValueError): - self.fit.result[0].to_table() - @requires_dependency('iminuit') - def test_basic_results_iminuit(self): + def test_basic_results(self): self.fit.method = 'iminuit' self.fit.fit() result = self.fit.result[0] @@ -238,8 +204,8 @@ def test_basic_errors(self): self.fit.fit() self.fit.est_errors() result = self.fit.result[0] - assert_allclose(result.model.parameters.error('index'), 0.09787747219456712) - assert_allclose(result.model.parameters.error('amplitude'), 2.1992645712596426e-12) + assert_allclose(result.model.parameters.error('index'), 0.0978669538795921) + assert_allclose(result.model.parameters.error('amplitude'), 2.199480205049676e-12) self.fit.result[0].to_table() def test_compound(self): @@ -247,10 +213,10 @@ def test_compound(self): fit.fit() result = fit.result[0] pars = result.model.parameters - assert_quantity_allclose(pars['index'].value, 2.2542315426423283) + assert_quantity_allclose(pars['index'].value, 2.254163434607357) # amplitude should come out roughly * 0.5 assert_quantity_allclose(pars['amplitude'].quantity, - 1.0243449507421302e-7 * u.Unit('m-2 s-1 TeV-1')) + 1.0338057602337021e-07 * u.Unit('m-2 s-1 TeV-1')) def test_npred(self): self.fit.fit() @@ -289,15 +255,6 @@ def test_fit_range(self): actual = self.fit.true_fit_range[0][0] assert_quantity_allclose(actual, desired) - @pytest.mark.xfail(reason='only simplex supported at the moment') - def test_fit_method(self): - self.fit.method_fit = "levmar" - assert self.fit.method_fit.name == "levmar" - self.fit.fit() - result = self.fit.result[0] - assert_quantity_allclose(result.model.parametersr['index'].value, - 2.2395184727047788) - def test_no_edisp(self): obs = self.obs_list[0] # Bring aeff in RECO space @@ -312,19 +269,26 @@ def test_no_edisp(self): 2.2960518556630887, atol=0.02) def test_ecpl_fit(self): + self.ecpl.parameters.set_parameter_errors( + {'amplitude' : 1e-11 * u.Unit('cm-2 s-1 TeV-1'), + 'lambda' : 0.1 / u.TeV} + ) fit = SpectrumFit(self.obs_list[0], self.ecpl) fit.fit() actual = fit.result[0].model.parameters['lambda_'].quantity - assert_quantity_allclose(actual, 0.0341911861834517 / u.TeV) + assert_quantity_allclose(actual, 0.0342866790304526 / u.TeV) def test_joint_fit(self): + self.pwl.parameters.set_parameter_errors( + {'amplitude' : 1e-11 * u.Unit('cm-2 s-1 TeV-1')} + ) fit = SpectrumFit(self.obs_list, self.pwl) fit.fit() actual = fit.result[0].model.parameters['index'].quantity - assert_quantity_allclose(actual, 2.212325780417152) + assert_quantity_allclose(actual, 2.212498637800248) actual = fit.result[0].model.parameters['amplitude'].quantity - assert_quantity_allclose(actual, 2.3621921135787887e-11 * u.Unit('cm-2 s-1 TeV-1')) + assert_quantity_allclose(actual, 2.364714363750943e-11 * u.Unit('cm-2 s-1 TeV-1')) def test_stacked_fit(self): stacked_obs = self.obs_list.stack() @@ -332,11 +296,14 @@ def test_stacked_fit(self): fit = SpectrumFit(obs_list, self.pwl) fit.fit() pars = fit.result[0].model.parameters - assert_quantity_allclose(pars['index'].value, 2.2132304579760893) + assert_quantity_allclose(pars['index'].value, 2.2133885226771) assert_quantity_allclose(pars['amplitude'].quantity, - 2.3618290865168973e-11 * u.Unit('cm-2 s-1 TeV-1')) + 2.3646703759892524e-11 * u.Unit('cm-2 s-1 TeV-1')) def test_run(self, tmpdir): + self.pwl.parameters.set_parameter_errors( + {'amplitude' : 1e-11 * u.Unit('cm-2 s-1 TeV-1')} + ) fit = SpectrumFit(self.obs_list, self.pwl) fit.run(outdir=tmpdir) diff --git a/gammapy/spectrum/tests/test_flux_point.py b/gammapy/spectrum/tests/test_flux_point.py index 7c81bf3cccb..3f0339585a6 100644 --- a/gammapy/spectrum/tests/test_flux_point.py +++ b/gammapy/spectrum/tests/test_flux_point.py @@ -152,6 +152,7 @@ def test_compute_flux_points_dnde_exp(method): @requires_dependency('matplotlib') @requires_dependency('scipy') @pytest.mark.parametrize('config', ['pl', 'ecpl']) +@pytest.mark.xfail(reason='This is rewritten in #1500') def test_flux_points(config): # TODO: replace this with a simple test case in a fixture filename = '$GAMMAPY_EXTRA/datasets/hess-crab4_pha/pha_obs23523.fits' diff --git a/gammapy/utils/fitting/iminuit.py b/gammapy/utils/fitting/iminuit.py index 734517cc95a..18ab760bfb7 100644 --- a/gammapy/utils/fitting/iminuit.py +++ b/gammapy/utils/fitting/iminuit.py @@ -45,7 +45,10 @@ def fit_iminuit(parameters, function, opts_minuit=None): **opts_minuit) minuit.migrad() - parameters.covariance = _get_covar(minuit) + if minuit.covariance is not None: + parameters.covariance = _get_covar(minuit) + else: + raise ValueError("No valid covariance matrix found") return parameters, minuit