seshnadathur / victor

Python code for likelihood analysis and MCMC posterior sampling of void-galaxy cross-correlation data
GNU General Public License v3.0
8 stars 7 forks source link

Change to iaH_true #15

Closed rdnv closed 1 year ago

rdnv commented 1 year ago

Add an apar rescaling to iaH in theory_xi() so that iaH_true is used

Define self.template_sigma8 in self.template_f in _set_velocity_pdf(). Add a growth_term = fs8_true / fs8_mock for mean_model='template'. Define iaH_mock in velocity_terms(). Add an apar rescaling to iaH so that velocity_terms() returns vr_true.

Close #14

seshnadathur commented 1 year ago

There are a bunch of things about this that I don't really agree with:

First up, I don't like requiring template_f to be specified in the input. We only require the template value of f*sigma8 in the normalisation, so that is what we should ask for. I think a better way to proceed is this:

  1. If model['velocity_pdf']['mean']['model'] is set to 'template' via the config file, we should require the user to specify the template value of fsigma8 (line 220) and throw an error if it is not provided. In this case we do not care about the template value of sigma8 alone at all so we should ignore it even if provided. It therefore makes sense to specify 'template_fsigma8' in the 'velocity_pdf sub-block of the config, while 'template_sigma8' is specified in the 'matter_ccf' sub-block as currently.
  2. In this scenario, we should also give the option to specify the redshift of the template simulation snapshot z_sim (and allow this to default to the effective redshift z_eff if not explicitly provided) and to provide the ratio of H_fiducial(z_eff)/H_sim(z_sim) – can be called something like 'template_hubble_ratio' in the config file for convenience of notation – which should default to 1 if not provided. I think it is much better to provide a ratio than the actual value of aH, the latter becomes confusing when you wonder if it has units or not, and which units!
  3. From z_sim, z_eff and 'Hubble_ratio' the code should then internally calculate the ratio of aH in the fiducial cosmology to its value in the template cosmology. This can then be used together with the value of a_par to change line 430 to read growth_term = (params['fsigma8'] / self.template_fsigma8) * self.template_hubble_ratio * (1 + self.z_sim) / (1 + self.z_eff) / a_par
  4. All the extra config options should be added to the example config file along with explanations about usage and default values, since this partially serves as a user guide

I also think there is a mistake in the computation of true velocity profile in line 474. I think this line should be vr = self.radial_velocity(r) * iaH_mock / iaH_true * growth_term using your notation and variables. If you use my suggested change to the definition of growth_term above, it would just be vr = self.radial_velocity(r) * growth_term.

seshnadathur commented 1 year ago

I created a new branch of my own to basically implement your changes but in the way that I wanted them done, and correcting a few minor errors on the way: https://github.com/seshnadathur/victor/tree/AP_scaling_velocity_templates

seshnadathur commented 1 year ago

I'm going to close this one now since the changes have all been incorporated into the other pull request