CFSv2: throw error on using --dFreq with --gridType={8,9}
Description
- closes #773 (closed)
- first commit: make CFSv2 throw error on using --dFreq with --gridType={8,9}
- just like for df{1,2,3}dot, this makes no sense, since the grid scaling is computed internally
- second commit: get the freq points and spacing from dopplerscan and lattice tiling modules
- new low-level
XLALLatticeTilingPointsAtDimension()
(equivalent toXLALTotalLatticeTilingPoints()
but at arbitrary dim)- new
XLALNumDopplerPointsAtDimension()
wrapping this and making it callable from e.g. CFSv2 - new
XLALGetDopplerLatticeTilingStepSize()
giving the closest equivalent of a grid step size from a lattice (not actually used, but useful to have for exposing the lower-levelXLALLatticeTilingStepSize()
) - use the number of points in CFSv2 to fix up the NFreq output in timing files
for
--gridType={8,9}
(callingXLALNumDopplerPointsAtDimension()
at dimension 2, which at least in the current setup always covers the frequency, with 0 and 1 being sky locations which are not tiled.)
- new
extra remarks:
- The "dim 2 is frequency" might not be globally robust, but at least the way CFSv2 is set up, it should be.
- Simply getting the "number of points" at that dimension and calling it the number of frequency points would break if there is also a sky grid, but the two grid types for which this mode is now called explicitly forbid multiple sky positions.
- As Karl pointed out, the "step size" concept is a bit fishy for nonsquare lattices in general, but again for the way things are set up, there should always be a single fixed step in frequency after all.
- Using the direct number of points call for the NFreq output is more robust than going via 1/stepSize, which seemed to pick up random-ish off-by-one errors for gridType 8 at least when I briefly tested it. (Which makes sense, since the lattice is not guaranteed to line up with the requested band boundaries.) Once coded up, I left that extra function in however, since it could be useful at least for developers / debuggers.
-
DopplerFullScan.[ch]
was already inconsistently putting its function documentations either on the prototypes or implementations, so in that respect I just followed the closest equivalents from which I copied, which ended up being in the two different styles.🤷 But the output should look the same.
API Changes and Justification
Backwards Compatible Changes
-
This change does not modify any class/function/struct/type definitions in a public C header file or any Python class/function definitions -
This change adds new classes/functions/structs/types to a public C header file or Python module
Backwards Incompatible Changes
-
This change modifies an existing class/function/struct/type definition in a public C header file or Python module -
This change removes an existing class/function/struct/type from a public C header file or Python module
If any of the Backwards Incompatible check boxes are ticked please provide a justification why this change is necessary and why it needs to be done in a backwards incompatible way.
Review Status
@karl-wette I'm relying on a somewhat phenomenological understanding of what the tiling module does, but while it's backed up by testing it quickly for some cases (for single-dim Freq-only tiling the --countTemplates
output agrees with the NFreq
field in the timing file, and when adding higher spindowns then the NFreq
still agrees with len(np.unique())
on the Freq column of the Fstat output file), so let me know if any conceptual mistakes are hiding in here. Also, since the detailed contents of the timing output file aren't part of the testsuite yet in general, I didn't add those checks explicitly anywhere, but it would be possible if necessary.