Skip to content

Commit

Permalink
Fixes #54
Browse files Browse the repository at this point in the history 
Added options allowing user to force update likelihood and avoid errors caused by unintended functioning of the automatic need-update-check.
  • Loading branch information
@thjsal @dhuppenkothen
thjsal authored and dhuppenkothen committed Sep 23, 2022
1 parent e7b567c commit 43edd8f
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 20 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ Attribution
^^^^^^^^^^^

[pre-v0.7.12] - 2022-09-15
~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~

Summary
^^^^^^^

* Since version 0.7.11. a few changes have been made including updates to the documentation and the handling of numerical problems in ray tracing. The latter fix can potentially have a small effect on the calculated pulse profiles and likelihood values for some parameter vectors, but according to the testing that effect is very minor.
* Since version 0.7.11. a few changes have been made including updates to the documentation and the handling of numerical problems in ray tracing. The latter fix can potentially have a small effect on the calculated pulse profiles and likelihood values for some parameter vectors, but according to the testing that effect is very minor at most.


Fixed
Expand All @@ -61,6 +61,8 @@ Added

* Added an option to force update ``xpsi/Star.py`` to avoid errors, for example, when all paremeters are fixed and X-PSI thinks otherwise that updating can be skipped (T.S., D.C., Y.K.).

* Added options allowing the user to truly force update the likelihood in ``xpsi/Likelihood.py`` and avoid errors caused by the automatic need-update-checks not working for all the possible cases. Added also an error message suggesting to use those options if the usual "AttributeError: 'CustomSignal' object has no attribute '_loglikelihood'" would be encountered (T.S.).

Changed
^^^^^^^

Expand Down
42 changes: 24 additions & 18 deletions xpsi/Likelihood.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,22 +294,21 @@ def _divide(obj, x):
else:
return None

def _driver(self, fast_mode=False, synthesise=False, **kwargs):
def _driver(self, fast_mode=False, synthesise=False, force_update=False, **kwargs):
""" Main likelihood evaluation driver routine. """

self._star.activate_fast_mode(fast_mode)

star_updated = False

if self._star.needs_update: # ignore fast parameters in this version
if self._star.needs_update or force_update: # ignore fast parameters in this version
try:
if fast_mode or not self._do_fast:
fast_total_counts = None
else:
fast_total_counts = tuple(signal.fast_total_counts for\
signal in self._signals)

self._star.update(fast_total_counts, self.threads)
self._star.update(fast_total_counts, self.threads,force_update=force_update)
except xpsiError as e:
if isinstance(e, HotRegion.RayError):
print('Warning: HotRegion.RayError raised.')
Expand Down Expand Up @@ -431,7 +430,7 @@ def __call__(self, p = None, reinitialise = False, force = False):
raise TypeError('Parameter values have not been updated.')
super(Likelihood, self).__call__(p) # update free parameters

if self.needs_update:
if self.needs_update or force:
try:
logprior = self._prior(p) # pass vector just in case wanted
except AttributeError:
Expand All @@ -445,17 +444,17 @@ def __call__(self, p = None, reinitialise = False, force = False):
if self._do_fast:
# perform a low-resolution precomputation to direct cell
# allocation
x = self._driver(fast_mode=True)
x = self._driver(fast_mode=True,force_update=force)
if not isinstance(x, bool):
super(Likelihood, self).__call__(self.cached) # restore
return x
elif x:
x = self._driver()
x = self._driver(force_update=force)
if not isinstance(x, bool):
super(Likelihood, self).__call__(self.cached) # restore
return x
else:
x = self._driver()
x = self._driver(force_update=force)
if not isinstance(x, bool):
super(Likelihood, self).__call__(self.cached) # restore
return x
Expand All @@ -466,7 +465,11 @@ def __call__(self, p = None, reinitialise = False, force = False):
loglikelihood = 0.0
for signals in self._signals:
for signal in signals:
loglikelihood += signal.loglikelihood
try:
loglikelihood += signal.loglikelihood
except AttributeError as e:
print("ERROR: It looks like X-PSI falsely thought that the signal does not need to be updated and thus skipped an essential part of the calculation. If not sampling, please use ``force=True`` or ``force_update=True`` option for the likelihood evaluation, or if sampling please set ``likelihood.externally_updated = True``")
raise

if loglikelihood <= self.llzero:
return self.random_near_llzero
Expand All @@ -486,7 +489,8 @@ def check(self,
logprior_call_vals=None,
rtol_logprior=None,
atol_logprior=None,
physical_points=None):
physical_points=None,
force_update=False):
""" Perform checks on the likelihood evaluator and the prior density.
Can be called from :func:`~.Sample.nested` to execute a check before
Expand All @@ -503,8 +507,10 @@ def check(self,
``m`` is dimensionality (``self.num_params``) of the sampling space.
The ``hypercube_points``, if not ``None``, will be ignored.
"""
:param optional[bool] force_update:
Force everything to be re-calculated regardless of what was computed before. This can be used to prevent errors in cases when the automatic check for update need is not working as intended.
"""
try:
from _np import allclose
except ImportError:
Expand Down Expand Up @@ -546,7 +552,7 @@ def allclose(a, b, rtol, atol, equal_nan=None):

self.externally_updated = False
for point in physical_points:
lls.append(self.__call__(point))
lls.append(self.__call__(point,force=force_update))
if lps is not None:
lps.append(self._prior(point))

Expand All @@ -572,7 +578,7 @@ def allclose(a, b, rtol, atol, equal_nan=None):

for point in hypercube_points:
phys_point = self._prior.inverse_sample(point)
lls.append(self.__call__(phys_point))
lls.append(self.__call__(phys_point,force=force_update))
if lps is not None:
lps.append(self._prior(phys_point))

Expand Down Expand Up @@ -620,7 +626,7 @@ def synthesise(self, p, reinitialise=False, force=False, **kwargs):
:param dict kwargs:
Keyword arguments propagated to custom signal synthesis methods.
Examples of such arguments include exposure times or
required total count numbers (see example notebooks0.
required total count numbers (see example notebooks).
"""
if reinitialise: # for safety if settings have been changed
Expand All @@ -634,7 +640,7 @@ def synthesise(self, p, reinitialise=False, force=False, **kwargs):
raise TypeError('Parameter values have not been updated.')
super(Likelihood, self).__call__(p) # update free parameters

if self.needs_update:
if self.needs_update or force:
try:
logprior = self._prior(p) # pass vector just in case wanted
except AttributeError:
Expand All @@ -650,17 +656,17 @@ def synthesise(self, p, reinitialise=False, force=False, **kwargs):
if self._do_fast:
# perform a low-resolution precomputation to direct cell
# allocation
x = self._driver(fast_mode=True)
x = self._driver(fast_mode=True,force_update=force)
if not isinstance(x, bool):
super(Likelihood, self).__call__(self.cached) # restore
return None
elif x:
x = self._driver(synthesise=True, **kwargs)
x = self._driver(synthesise=True,force_update=force, **kwargs)
if not isinstance(x, bool):
super(Likelihood, self).__call__(self.cached) # restore
return None
else:
x = self._driver(synthesise=True, **kwargs)
x = self._driver(synthesise=True,force_update=force, **kwargs)
if not isinstance(x, bool):
super(Likelihood, self).__call__(self.cached) # restore
return None

0 comments on commit 43edd8f

Please sign in to comment.