Skip to content

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 to XLALTotalLatticeTilingPoints() 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-level XLALLatticeTilingStepSize())
    • use the number of points in CFSv2 to fix up the NFreq output in timing files for --gridType={8,9} (calling XLALNumDopplerPointsAtDimension() 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.)

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.

Merge request reports

Loading