added milky way height and radius distributions #4920
Conversation
pycbc/distributions/__init__.py
Outdated
| @@ -39,6 +39,10 @@ | |||
| from pycbc.distributions.fixedsamples import FixedSamples | |||
| from pycbc.distributions.mass import MchirpfromUniformMass1Mass2, \ | |||
| QfromUniformMass1Mass2 | |||
| from pycbc.distributions.milky_way_radial import MilkyWayRadial | |||
There was a problem hiding this comment.
Should be more generic than milkyway, name this after the distribution itself
| the config file should be | ||
| [prior-z] | ||
| name = milky_way_height | ||
| min-z = |
There was a problem hiding this comment.
Examples should work so give number
| ## _scale keys removed from the params. | ||
| ## Now pass to bounded.dist | ||
| super(MilkyWayHeight, self).__init__(**params) | ||
| ## raising error if scale provided for param not in kwargs |
There was a problem hiding this comment.
Remove old code, but do have comments that explain what you are doing
| z_weight3 = | ||
|
|
||
| ''' | ||
| def __init__(self, **params): |
There was a problem hiding this comment.
It's super unclear what the argument / keywords you are using
There was a problem hiding this comment.
Be explicit wherever possible, and use names that are straightforward
There was a problem hiding this comment.
It's not even clear what the purpose of scale 1 / scale 2 / scale 3 is?
| self._bounds = {} | ||
| self._scale1 = {} | ||
| self._scale2 = {} | ||
| self._scale3 = {} |
There was a problem hiding this comment.
Why are you limitted to exactly three disks? You have a lot of extra redundant code to handle essentially the same exact operations.
| self._norm[p] = 1. | ||
| self._lognorm[p] = -numpy.inf | ||
|
|
||
| @property |
There was a problem hiding this comment.
Don't include anything that's not required
|
|
||
| def _logpdf(self, **kwargs): | ||
| if kwargs in self: | ||
| return sum([numpy.log((self._weight1[p]/(2*self._scale1[p]*(1 - numpy.e**(self._bounds[p][0]/self._scale1[p]))))*numpy.e**(-numpy.abs(kwargs[p])/self._scale1[p]) |
| _cdf = numpy.zeros(len(param_array)) | ||
| _cdf[mask] = (beta1)*(numpy.e**(param_array[mask]/z_d1) - numpy.e**(z_min/z_d1)) + (beta2)*(numpy.e**(param_array[mask]/z_d2) - numpy.e**(z_min/z_d2)) + (beta3)*(numpy.e**(param_array[mask]/z_d3) - numpy.e**(z_min/z_d3)) | ||
| _cdf[~mask] = (beta1)*(2 - (numpy.e**(z_min/z_d1) + numpy.e**(-param_array[~mask]/z_d1))) + (beta2)*(2 - (numpy.e**(z_min/z_d2) + numpy.e**(-param_array[~mask]/z_d2))) + (beta3)*(2 - (numpy.e**(z_min/z_d3) + numpy.e**(-param_array[~mask]/z_d3))) | ||
| _cdfinv = scipy.interpolate.interp1d(_cdf, param_array) |
There was a problem hiding this comment.
Can you not directly solve for the inverse in this case? Why are interpolating?
| r_min, r_max = bounds | ||
| #if r_min < 0: | ||
| #raise ValueError(f'Minimum value of {p} must be greater than or equal to zero ') | ||
| r = numpy.linspace(r_min, r_max, 1000) |
There was a problem hiding this comment.
This is hardcoded, How do you we know you have enough samples here?
…ric gamma distribution)
Added two distributions
that samples the radius and height in galactocentric co-ordinates for the milky way 3 disc exponential model