Refactor for SoLikeT #86
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #86 +/- ##
=========================================
Coverage ? 85.24%
=========================================
Files ? 3
Lines ? 454
Branches ? 0
=========================================
Hits ? 387
Misses ? 67
Partials ? 0
|
Tt te ee + Cobaya 3.5.4
| logp += self.logp_const | ||
| self.log.debug( | ||
| f"Log-likelihood value computed = {logp} (Χ² = {-2 * (logp - self.logp_const)})" | ||
| f"Log-likelihood value computed = {logp} (Χ² = {-2 * (- self.logp_const)})" |
There was a problem hiding this comment.
I think the second logp has been lost when computing the chi-square value. Maybe we can write it
chi2 = self._fast_chi_squared(self.inv_cov, delta)
logp = -0.5 * chi2 + self.logp_const
self.log.debug(f"Log-likelihood value computed = {logp} (Χ² = {chi2})")| if ell is None: | ||
| ell = self.ells | ||
| model = self._get_foreground_model_arrays(fg_params, ell=ell) | ||
| experiments = self.experiments if freqs_order is None else freqs_order |
There was a problem hiding this comment.
experiments = freqs_order or self.experimentsis simpler I guess
| # get total foregrounds; model is dictionary of arrays for each frequency combo | ||
| model = self._get_foreground_model_arrays(params_values_dict) | ||
| return [np.sum([model[s, comp] for comp in self.fg_component_list[s]], axis=0) | ||
| for s in (requested_cl if requested_cl else self.requested_cls)] |
There was a problem hiding this comment.
Here also can be written
for s in requested_cl or self.requested_clsit's up to you.
I wonder if it is better (or not) to set the requested_cl function argument to None or leave it as empty tuple (the above code works in both cases)
| return self.current_state["fg_totals"] | ||
|
|
||
| def must_provide(self, **requirements): | ||
| if (req := requirements.get("fg_totals")) is not None: |
There was a problem hiding this comment.
if req := requirements.get("fg_totals"):| # Foreground requires bandint_freqs from BandPass | ||
| # TODO: not clear that changing number of freqs actually works (for shift parameters) | ||
| super().must_provide(**requirements) | ||
| if (req := requirements.get("fg_totals")) is not None: |
|
Agree on logp. I try to avoid comparing sequences that might be an np.ndarray, because bool(ndarray) raises an exception if the array is not empty, hence explicit comparisons to None |
Currently there are two different versions of MFLike (on separate git repos), SOLikeT and this, which is confusing and hard to maintain. This attempts to merge the two, with the objective of deleting the version in SOLikeT so we don't have any code duplication.
Basically it splits of BandpowerForeground as a separate Theory, allowing the foreground model to be swapped in and out, and the foreground and calibration/systematic parameters to be sampled separately. At the moment it doesn't split into a separate bandpower theory as in the current SOLikeT, and here TheoryForge obolished by replacing with the foreground model or merging into mflike.
pspipe_utils can now use BandpowerForeground instead of TheoryForge as in simonsobs/pspipe_utils#53.