Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backwards compatability breaks on master #475

Open
tilmantroester opened this issue Dec 3, 2024 · 1 comment
Open

Backwards compatability breaks on master #475

tilmantroester opened this issue Dec 3, 2024 · 1 comment
Labels

Comments

@tilmantroester
Copy link
Contributor

I've tried to run existing code on the current master branch and ran into a range of issues.

One repeating issue is the strict expectations that ells are integer. That is not true in practice, since any realistic measurement is going to be binned, with a window function and effective ell-label for the bin. (Formally we are working on a subset of the sphere, where the exact harmonic basis does not have integer ell).

E.g.: non-integer harmonic space scale cuts:
firecrown/likelihood/two_point.py:462: assert isinstance(self.theory.ell_or_theta_max, (int, type(None)))

This also pops up when creating the interpolation ells in LogLinearElls

Next issue is that the logic for bandpower window functions is broken again:

File [~/Codes/firecrown/firecrown/likelihood/two_point.py:464](https://vscode-remote+jupyter-002eeuler-002ehpc-002eethz-002ech.vscode-resource.vscode-cdn.net/cluster/home/tilmant/Research/LSST/baryon-challenge/notebooks/~/Codes/firecrown/firecrown/likelihood/two_point.py:464), in TwoPoint.read_harmonic_space(self, sacc_data)
    458 Cells, ells, sacc_indices, window = self.read_harmonic_spectrum_data(
    459     ells_cells_indices, sacc_data
    460 )
    461 # assert isinstance(self.theory.ell_or_theta_min, (int, type(None)))
    462 # assert isinstance(self.theory.ell_or_theta_max, (int, type(None)))
--> 464 ells, Cells, sacc_indices = gen.apply_ells_min_max(
    465     ells,
    466     Cells,
    467     sacc_indices,
    468     self.theory.ell_or_theta_min,
    469     self.theory.ell_or_theta_max,
    470 )
    471 self.theory.ells = ells
    472 if self.theory.ell_or_theta_min is not None:

File [~/Codes/firecrown/firecrown/generators/two_point.py:193](https://vscode-remote+jupyter-002eeuler-002ehpc-002eethz-002ech.vscode-resource.vscode-cdn.net/cluster/home/tilmant/Research/LSST/baryon-challenge/notebooks/~/Codes/firecrown/firecrown/generators/two_point.py:193), in apply_ells_min_max(ells, Cells, indices, ell_min, ell_max)
    191 locations = np.where(ells <= ell_max)
    192 ells = ells[locations]
--> 193 Cells = Cells[locations]
    194 if indices is not None:
    195     indices = indices[locations]

IndexError: index 20 is out of bounds for axis 0 with size 20

This can be traced back toapply_ells_min_max: it needs to use the ell-labels from the sacc data for the scale cuts, so this fails because ells here is the integer-spaced array, while Cells is the (binned) data vector

        ells, Cells, sacc_indices = gen.apply_ells_min_max(
            ells,
            Cells,
            sacc_indices,
            self.theory.ell_or_theta_min,
            self.theory.ell_or_theta_max,
        )

Next:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
File [~/Codes/firecrown/firecrown/updatable.py:192](https://vscode-remote+jupyter-002eeuler-002ehpc-002eethz-002ech.vscode-resource.vscode-cdn.net/cluster/home/tilmant/Research/LSST/baryon-challenge/notebooks/~/Codes/firecrown/firecrown/updatable.py:192), in Updatable.update(self, params)
    191 try:
--> 192     value = params.get_from_full_name(parameter.fullname)
    193 except KeyError as exc:

File [~/Codes/firecrown/firecrown/parameters.py:98](https://vscode-remote+jupyter-002eeuler-002ehpc-002eethz-002ech.vscode-resource.vscode-cdn.net/cluster/home/tilmant/Research/LSST/baryon-challenge/notebooks/~/Codes/firecrown/firecrown/parameters.py:98), in ParamsMap.get_from_full_name(self, full_name)
     96         return self[full_name_lower]
---> 98 raise KeyError(f"Key {full_name} not found.")

KeyError: 'Key Omega_c not found.'

The above exception was the direct cause of the following exception:

MissingSamplerParameterError              Traceback (most recent call last)
Cell In[9], line 33
     30 tools.reset()
     32 likelihood.update(params)
---> 33 tools.update(params)
     34 tools.prepare(ccl_cosmo)
     36 chi2 = likelihood.compute_chisq(tools)

File [~/Codes/firecrown/firecrown/updatable.py:198](https://vscode-remote+jupyter-002eeuler-002ehpc-002eethz-002ech.vscode-resource.vscode-cdn.net/cluster/home/tilmant/Research/LSST/baryon-challenge/notebooks/~/Codes/firecrown/firecrown/updatable.py:198), in Updatable.update(self, params)
    195     setattr(self, parameter.name, value)
    197 for item in self._updatables:
--> 198     item.update(params)
    200 self._update(params)
    201 # We mark self as updated only after all the internal updates have
    202 # worked.

File [~/Codes/firecrown/firecrown/updatable.py:194](https://vscode-remote+jupyter-002eeuler-002ehpc-002eethz-002ech.vscode-resource.vscode-cdn.net/cluster/home/tilmant/Research/LSST/baryon-challenge/notebooks/~/Codes/firecrown/firecrown/updatable.py:194), in Updatable.update(self, params)
    192         value = params.get_from_full_name(parameter.fullname)
    193     except KeyError as exc:
--> 194         raise MissingSamplerParameterError(parameter.fullname) from exc
    195     setattr(self, parameter.name, value)
    197 for item in self._updatables:

MissingSamplerParameterError: The parameter `Omega_c` is required to update something in this likelihood.
It should have been supplied by the sampling framework.
The object being updated was:
Omega_c

Here the issue is that ModellingTools.prepare does not take a ccl.Cosmology any more and forces the use of the CCLFactory.

After that there are issues again due to the wrong ells around apply_ells_min_max, which I couldn't hack around.

@tilmantroester
Copy link
Contributor Author

This is the context of the baryon challenge infrastructure. E.g. https://github.com/LSSTDESC/baryon-challenge/blob/main/notebooks/run_model_predictions.ipynb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant