Inconsistent use of cosmo in HMCalculator, MassFunc, HaloBias

[nikfilippas]

A Cosmology object is needed to initialize an instance of the HMCalculator. This is used to calculate rho0 and store it as a class attribute. Later on, cosmo is required when calling any of the HMCalculator methods.

If users initialize hmc with one cosmology, and then call it with a different cosmology CCL will yield inconsistent results without any errors.

A solution is to remove cosmo from a required argument of HMCalculator.__init__, and allow it to be instantiated without setting rho0 as an attribute. This quantity can be calculated on the fly whenever _get_ingredients is called (which is the only case where rho0 is used).

ref code

class HMCalculator(object):
    ...

    def __init__(self, cosmo, ...):
        self._rho0 = rho_x(cosmo, 1., 'matter', is_comoving=True)  # only time cosmo is used in init
        ...

    def _get_ingredients(self, a, cosmo, get_bf):
        # rho0 is only used in here
        self.mf0 = (self._rho0 - ... ... ...)
        self.mbf0 = (self._rho0 - ... ... ...)

[nikfilippas]

This will break API so although it's an easy fix, I think it needs to be tagged for v3.

[nikfilippas]

UPDATE
Having a closer look at HMCalculator.

Suppose we completely get rid of cosmo during initialization of the class instance. This still does not solve the problem, because we need to pass instances of MassFunc and HaloBias, which will have been defined for a specific cosmology!

Then, if we call _get_ingredients for a different cosmo than the one we used to instantiate our mass function and halo bias, we will once again get inconsistent results.

There are two solutions to this issue:

1. Either leave cosmo as it is, in which case, for functions that take both cosmo and hmc this will be like we pass cosmo twice (since only one cosmo is valid for a given hmc);
2. Or pass the mass function and halo bias classes (instead of instances) into HMCalculator.__init__, in which case, the mass function and the halo bias will be re-computed every time.

Both of these point to putting HMCalculator under Cosmology, since with (1) there is a one-to-one correspondence of cosmo instance to hmc instance, and with (2) we do not end up saving time computing the mass function and halo bias, even if we avoid passing cosmology in the __init__ method of HMCalculator.

Any thoughts are welcome. (tagging @damonge because you were reluctant to put hmc under cosmo)

[nikfilippas]

So what I propose is, in addition to the mass_function argument of Cosmology, we add halo_bias, mass_def.
(I realize mass_function is deprecated, but we'll keep it there for a different reason.)
These will be tied to the Cosmology instance. I don't think anyone will want to change their mass function, halo bias, or mass definition in the middle of an MCMC run, for example, so it's probably fine if these, just like the cosmological parameters, are also immutable.
It also kind of makes sense to have them as Cosmology attributes as they have more to do with the universe as a whole, and its phenomenology, than with the specific model we use.
And this way, we also avoid having to pass both cosmo and hmc in many functions of pyccl, simplifying their signature and the pipelines created with them!

[nikfilippas]

UPDATE
I have been looking at this for a little while now.

HMCalculator requires that we pass an instance of a mass function. All mass functions take cosmo as their first argument, and they compute the sigma(M) splines.

Later, when any of the HMCalculator methods are called, cosmo is passed in massfunc.get_mass_function, which in turn passes it to (or has access to):

1. massfunc._get_consistent_mass --> this is OK if the mass definition does not change
2. ccllib.sigM_vec
3. ccllib.dlnsigM_dlogM_vec
4. cosmo['Omega_m']
5. cosmo['h']
6. _get_fsigma --> this takes cosmo but does not always use it

Apart from (1) and (6), all others will give inconsistent results when called with a different cosmo to the one the mass function was instantiated.

In the end, the actual calculation isn't affected significantly, but it would be nice to have a consistent way of dealing with different cosmological parameters.

[damonge]

I'd much rather remove cosmo as an argument when initializing the mass functions and halo biases than clutter cosmology objects with anything halo-model related. This is definitely doable for all the mass functions and biases we have implemented.

[nikfilippas]

Yes, exactly, I was about to open a PR. I've already done that for the mass functions and the halo biases.
That's one way to do it. The other way would be to implement it under Cosmology (which you don't like), in order to make the signature of functions that take (cosmo, hmc) simpler. But if we want to avoid putting HMCalculator under Cosmology at all costs, then the only solution I see is removing cosmo from the __init__ method of all the mass functions and halo biases.