dingo-gw / dingo

Dingo: Deep inference for gravitational-wave observations
MIT License
55 stars 18 forks source link

Division By 0 in standardization causes issues #133

Closed nihargupte-ph closed 1 year ago

nihargupte-ph commented 1 year ago

When setting a delta function prior such as bilby.core.prior.analytical.DeltaFunction(0.0) there is a division by zero error in line 103 of dingo.gw.transforms.parameter_transforms.py. This happens because when calling the SelectStandardizeRepackageParameters transform, we divide by the standard deviation of a set of parameters. This should only really happen when all parameter samples are 0 (ie a delta function). A simple fix is to add an edge case when all parameter samples are 0. In this case, the samples would be standardized to 0.

stephengreen commented 1 year ago

How did this error come up? Usually one would not want to infer a quantity that is fixed by the prior to a specific value.

nihargupte-ph commented 1 year ago

Right, I was setting the eccentricity to 0 in SEOBNRv4E. Eventually this is so that we can compare how the model performs when no eccentricity is allowed (computing Bayes factors as I believe Max and Michael suggested). This should reduce to SEOBNRv4 but just in case there are other differences between the waveform models, this gives us a more direct comparison.

stephengreen commented 1 year ago

When fixing the eccentricity to 0, you should also exclude it from the list of inference_parameters. Setting aside the standardisation issue, a normalising flow will in any case have trouble learning a delta function. It also doesn't provide any new information, so it makes sense to just not infer it.

mpuerrer commented 1 year ago

Similarly, you would also want to exclude the anomaly (eccentric radial phase parameter) when the eccentricity is excluded.

nihargupte-ph commented 1 year ago

Ah right, good point. In that case, I will close this issue.