Hey @dfm, the periodogram test is passing now! There were three issues: the first was that using f0=0 (i.e. starting the frequency grid at exactly zero) was yielding power[0] = nan, from both Astropy and the C++ code. I added an f0 parameter to the C++ code and set it to df/2, which is the Astropy default. Or we could just special-case power[0] if we know we want f0=0.
The second issue was overall normalization of the power, so I implemented the Astropy "standard" normalization in the C++ module.
The third issue was that the Astropy code was normalizing w = w/w.sum() (where w is the inverse error 1/dy), but the C++ code wasn't. I didn't implement that in the C++ code because it requires a new array allocation (or overwriting the input), so for now we just require w to be pre-normalized when passed to C++.
dfm
commented
2 years ago
Hey @dfm, the periodogram test is passing now! There were three issues: the first was that using
f0=0(i.e. starting the frequency grid at exactly zero) was yieldingpower[0] = nan, from both Astropy and the C++ code. I added anf0parameter to the C++ code and set it todf/2, which is the Astropy default. Or we could just special-casepower[0]if we know we wantf0=0.The second issue was overall normalization of the power, so I implemented the Astropy "standard" normalization in the C++ module.
The third issue was that the Astropy code was normalizing
w = w/w.sum()(wherewis the inverse error1/dy), but the C++ code wasn't. I didn't implement that in the C++ code because it requires a new array allocation (or overwriting the input), so for now we just requirewto be pre-normalized when passed to C++.I'll start looking at benchmarking next!