Realization from Galacticus output #43
Conversation
Included is also various classes for reading galacticus output
Fixed a bug with rotation.
There was a problem hiding this comment.
Hi @cgannonucm, looks like a reasonable implementation to me! I left a few comments just on the new Halo class for now.
First, since some of the methods are identical to the other TNFW halo class I think you should have this class be a subclass of TNFW, i.e. TNFWFromParams(TNFW) instead of TNFWFromParams(Halo). That way you don't have to copy the same method into different classes. Note that you will have override some methods in the new class, in particular the params_physical calculation, calculating the concentration, and the lenstronomy_params method.
On the last point, you'll need to compute theta_Rs and Rs_angle directly from params physical rather than using the mass and concentration. You can use the methods in lens_cosmo for this
|
|
||
| y = y_kpc / self._kpc_per_arcsec_at_z | ||
|
|
||
| self._params_physical = args |
There was a problem hiding this comment.
In general args might have other parameters besides the physical parameters describing the NFW profile. Can you extract only the parameters you need from args to define self._params_physical?
| x = r / r_s | ||
| tau = r_t / r_s | ||
|
|
||
| n = 0 |
There was a problem hiding this comment.
Good catch, this was left in from when I was running some checks. Will remove.
| """ | ||
| if not hasattr(self, '_kwargs_lenstronomy'): | ||
| [concentration, rt] = self.profile_args | ||
| Rs_angle, theta_Rs = self._lens_cosmo.nfw_physical2angle(self.mass, concentration, self.z) |
There was a problem hiding this comment.
This method will have to change to compute Rs_angle and theta_Rs directly from self._params_physical
…dels/TNFWFromParams Other changes - General improvements to code / documentation - Changed the way subhalo coordinates are projected
|
Hi @cgannonucm, looks good to me. For some reason your pull request doesn't trigger tests and the coveralls run, I'm not sure why. In the meantime, it looks like there are some merge conflicts with the main branch that need to be resolved. In particular, the preset model functions are now all distributed in their files in the PresetModels class folder -- you can put this one in the external.py file. Maybe if you fix the merge conflicts it will trigger the tests and coverage runs..... |
|
Hi @dangilman This branch and the main branch should be merged now, and all conflicts resolved. |
|
@cgannonucm thanks! It looks like there is a problem with one of the test functions for a different method that is causing the tests to fail, would you mind fixing it so that the coverage and tests run? You just have to change the function names in the import statement of test_realization_extensions.py from _xi_l, _xi_l_to_Pk_l to "xi_l" and "xi_l_to_Pk_l". Then it should work |
|
@dangilman Done. The tests should run now. |
Implements the ability to create a realization from a Galacticus output file.