prashjet / popkinmocks

Create mock IFU datacubes for stellar population and kinematic modelling.
https://popkinmocks.readthedocs.io/
MIT License
4 stars 1 forks source link

get_correlation('zv_x') error #84

Closed margitschmid123 closed 1 year ago

margitschmid123 commented 1 year ago

the statement: corr_zv_x_s = simulation.get_correlation('zv_x')

gives the following error:

File "/home/margit/popkinmocks/popkinmocks/components/base.py", line 487, in get_correlation covariance = self.get_covariance(which_dist, light_weighted=light_weighted) File "/home/margit/popkinmocks/popkinmocks/components/base.py", line 445, in get_covariance integrand = p * delta_var0[:, na] * delta_var1[na, :] ValueError: operands could not be broadcast together with shapes (25,6,51,51) (6,1,51,51)

the correlation with age corr_tv_x_s = simulation.get_correlation('tv_x') works fine, also 'tz_x' works for my example:

ssps = pkm.milesSSPs(thin_age=6, thin_z=2) cube = pkm.ifu_cube.IFUCube( ssps=ssps, nx1=51, nx2=51, nv=25, vrng=(-1000, 1000), x1rng = (-20, 20), x2rng = (-20, 20) ) simulation = pkm.components.FromParticle(cube, ages, vel[:, 2], coords[:, 0], coords[:, 1], MtoH)

prashjet commented 1 year ago

Thanks Margit. I found the problem - the string which defines the distributions have to be in alphabetical order on either side of the _. So instead of

corr_zv_x_s = simulation.get_correlation('zv_x')

it should be

corr_vz_x_s = simulation.get_correlation('vz_x')

i.e. with the order of v and z flipped.

It shouldn't matter if we ask for the correlation between z and v, or between v and z - they are the same quantity. But the way things are implemented in popkinmocks, we need the strings to be in alphabetical order. I've been meaning to add a warning if you try to use a non-alphabetical string, and this issue reminds me why this is super important. But for the time being please se the fix above.

I will close this issue note in the existing issue once this is done.