Refactoring SLSim - Merging DeflectorBase and SourcePopBase into single Population class.

[jacob-hjortlund]

Draft PR for #225.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 141 lines in your changes missing coverage. Please review.

Project coverage is 94.50%. Comparing base (01736c4) to head (c42da9b).

Files	Patch %	Lines
slsim/Populations/populations.py	0.00%	101 Missing ⚠️
slsim/Populations/velocity_dispersion.py	0.00%	40 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #237      +/-   ##
==========================================
- Coverage   97.63%   94.50%   -3.13%     
==========================================
  Files          66       68       +2     
  Lines        4264     4405     +141     
==========================================
  Hits         4163     4163              
- Misses        101      242     +141     

Files	Coverage Δ
slsim/Populations/velocity_dispersion.py	0.00% <0.00%> (ø)
slsim/Populations/populations.py	0.00% <0.00%> (ø)

[jacob-hjortlund]

@sibirrer @nkhadka21 is it correctly understood that in the source Galaxies class and Elliptical/AllLensGalaxies class we have ellipticies given by e1/e2 and e1_light / e2_light assuming a single sersic profile, respectively? Just want to make sure such that naming of galaxy properties can be standardised between source and lens galaxies.

[sibirrer]

@sibirrer @nkhadka21 is it correctly understood that in the source Galaxies class and Elliptical/AllLensGalaxies class we have ellipticies given by e1/e2 and e1_light / e2_light assuming a single sersic profile, respectively? Just want to make sure such that naming of galaxy properties can be standardised between source and lens galaxies.

Hi @jacob-hjortlund the eccentricities in mass and light are defined in the same way. They may not necessarily imply Sersic profiles (ellipticity distortions are general as long as its constant as a function of radius). Where you have to pay attention is between eccentricities in light and mass, and these should be held differently (at least for the power-law mass density profile)

[jacob-hjortlund]

@sibirrer @nkhadka21 is it correctly understood that in the source Galaxies class and Elliptical/AllLensGalaxies class we have ellipticies given by e1/e2 and e1_light / e2_light assuming a single sersic profile, respectively? Just want to make sure such that naming of galaxy properties can be standardised between source and lens galaxies.

Hi @jacob-hjortlund the eccentricities in mass and light are defined in the same way. They may not necessarily imply Sersic profiles (ellipticity distortions are general as long as its constant as a function of radius). Where you have to pay attention is between eccentricities in light and mass, and these should be held differently (at least for the power-law mass density profile)

Thanks, makes sense. Tiny follow-up: Assuming we define ellipticity as q^2 = ( 1 - \epsilon ) / ( 1 + \epsilon ), shouldn't eccentricity be given by e = \sqrt( 1 - q^2) = \sqrt( 2 \epsilon / ( 1 + \epsilon ) ) instead of e = ( 1 - \epsilon ) / ( 1 + \epsilon )? Is a different definition of eccentricity being used or am I just bad at arithmetic haha

[sibirrer]

@sibirrer @nkhadka21 is it correctly understood that in the source Galaxies class and Elliptical/AllLensGalaxies class we have ellipticies given by e1/e2 and e1_light / e2_light assuming a single sersic profile, respectively? Just want to make sure such that naming of galaxy properties can be standardised between source and lens galaxies.

Hi @jacob-hjortlund the eccentricities in mass and light are defined in the same way. They may not necessarily imply Sersic profiles (ellipticity distortions are general as long as its constant as a function of radius). Where you have to pay attention is between eccentricities in light and mass, and these should be held differently (at least for the power-law mass density profile)

Thanks, makes sense. Tiny follow-up: Assuming we define ellipticity as q^2 = ( 1 - \epsilon ) / ( 1 + \epsilon ), shouldn't eccentricity be given by e = \sqrt( 1 - q^2) = \sqrt( 2 \epsilon / ( 1 + \epsilon ) ) instead of e = ( 1 - \epsilon ) / ( 1 + \epsilon )? Is a different definition of eccentricity being used or am I just bad at arithmetic haha

Hi @jacob-hjortlund: we are using the definition e = (1-q)/ (1+q). In that definition, the distortions are equivalent with reduced shear components

[jacob-hjortlund]

@sibirrer @nkhadka21 currently, if velocity dispersions are not provided in the input table for the Elliptical / AllLensGalaxies classes the vel_disp column is set to -1 and filled in later. This filling is done using the vel_disp_abundance_matching and vel_disp_from_m_star functions, but it seems the latter is actually never used. This is due to vel_disp_abundance_matching using fill values (0, np.max(galaxy_list_zmax["vel_disp"])) for any log10 m_star values outside the range used to create the 1d interpolator, meaning after we apply vel_disp_abundance_matching and the resulting _f_vel_disp function during initialization, we ensure that there are never any instances of vel_disp=-1 to be tackled by vel_disp_from_m_star.

Is this the intended behaviour? If so, we should simply remove vel_disp_from_m_star. If not, what edge case would you expect this function to fill? From what I can see in the reference, this relation applies to mid/high mass ellipticals. Currently vel_disp_abundance_matching spits out an interpolator between mass and velocity dispersion for the highest mass galaxies below the redshift cutoff (fixed to z=0.5 currently). If we have massive galaxies larger than that found in the input catalog below z=0.5, would it make more sense to use the empirical scaling relation?

[nkhadka21]

@sibirrer @nkhadka21 currently, if velocity dispersions are not provided in the input table for the Elliptical / AllLensGalaxies classes the vel_disp column is set to -1 and filled in later. This filling is done using the vel_disp_abundance_matching and vel_disp_from_m_star functions, but it seems the latter is actually never used. This is due to vel_disp_abundance_matching using fill values (0, np.max(galaxy_list_zmax["vel_disp"])) for any log10 m_star values outside the range used to create the 1d interpolator, meaning after we apply vel_disp_abundance_matching and the resulting _f_vel_disp function during initialization, we ensure that there are never any instances of vel_disp=-1 to be tackled by vel_disp_from_m_star.

Is this the intended behaviour? If so, we should simply remove vel_disp_from_m_star. If not, what edge case would you expect this function to fill? From what I can see in the reference, this relation applies to mid/high mass ellipticals. Currently vel_disp_abundance_matching spits out an interpolator between mass and velocity dispersion for the highest mass galaxies below the redshift cutoff (fixed to z=0.5 currently). If we have massive galaxies larger than that found in the input catalog below z=0.5, would it make more sense to use the empirical scaling relation?

Hi @jacob-hjortlund, Thank you for asking about this! We were using the vel_disp_from_m_star function to compute the velocity dispersion of elliptical lens galaxies. However, we later discovered that the resulting lens population had an unusual velocity dispersion distribution. Therefore, we switched to using the SDSS velocity dispersion function for elliptical galaxies as well.

If we use vel_disp_from_m_star for galaxies with a stellar mass higher than the range used in the interpolation, there's a chance we might end up with more than 20,000 galaxies with very high velocity dispersion. This could lead to an overprediction of the lens population.
So, I suggest moving this function to astro_util so that we can use it later if needed. Thank you!