Set default values of constants in white_noise_block

[mattpitkin]

Currently, in the white_noise_block function, if vary=False the efac, equad and ecorr values are set as Constant parameters without a given value for that constant. This means that the constant value will default to None. The docstring suggests that the constant values can be set later, but in a function such as model_singlepsr_noise, if white_vary=False these constants don't get set and subsequent likelihood calls on the returned pta object fail due to the constants returning None.

This PR attempts to fix this by setting some default values for the constants. In this case a value of 1.0 for efac and -inf for both equad and ecorr. This (assuming I understand things correctly!) would mean that the likelihood just uses the TOA errors as given as the standard deviations in the likelihood calculation (without any scaling, additional component, or correlation).

See also nanograv/enterprise#379.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.29%. Comparing base (d91baf2) to head (70f9434).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #226   +/-   ##
=======================================
  Coverage   37.29%   37.29%           
=======================================
  Files          20       20           
  Lines        3974     3974           
=======================================
  Hits         1482     1482           
  Misses       2492     2492           

Files	Coverage Δ
enterprise_extensions/blocks.py	44.41% <100.00%> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d91baf2...70f9434. Read the comment docs.

[vhaasteren]

Hi @mattpitkin,

Sorry you do not get much of a response here. Although I don't really use (or maintain?) e_e, it seems I'm the only one here regardless.

Two remarks. Firstly, it can also be thought of as a feature that Enterprise will fail if the white noise parameters are not set. In Enterprise, the (white) noise parameters are set with the function set_default_params, which takes a dictionary with white noise values. It is very easy to accidentally have the wrong white noise dictionary. For example, if you accidentally use TNEfac instead of T2Efac for some pulsars, the noise dictionary would be incorrect. Nice thing now is that you then get an error. With this PR, no error would be thrown, and the analysis would be incorrect. Worse, the user would not even know about it.

I understand your desire to have the white noise parameters default to efac=1. But because of the above, I would not just change the defaults. It's dangerous, and someone somewhere sometime will make a pretty nasty mistake. Perhaps you can modify the PR so that there is an option? May set vary='efac_is_one' or something more elegant for that?

Secondly, is Enterprise happy with the -np.inf? Will it just run without problems?

Again, apologies that you don't get a response here.

[mattpitkin]

Hi @vhaasteren, thanks for having a look at this. As you suggested, I've updated the PR so that you can now instead provide some default constant values via keyword arguments, but otherwise it will function as before.

[vhaasteren]

Thanks @mattpitkin , this looks fine. I'm tempted to just merge, but perhaps better to still wait for tests to pass. The reason the tests fail at the moment are because of libstempo/tempo2 actually. In PR #248 I made a change that will make libstempo/tempo2 optional, so then the CI will run, but someone will need to approve it first.

Actually, since you do some libstempo maintenance, do you know how to fix that?