Skip to content

Cleanup spec avg long

Evan Goetz requested to merge evan-goetz/lalsuite:cleanup-spec_avg_long into master

Description

Summary

lalapps_spec_avg_long is a program that takes in SFTs, averages them together using both arithmetic and noise-weighted averages. This is based loosely on the lalapps_spec_avg program. lalapps_spec_avg_long.c had a lot of commented out code, user-input arguments that were entirely unused, an ancient SFT reading method, and the output saved 3 0-byte files. In addition, it did not properly free all allocated arrays at the end of the program. I have cleaned up the code for the most part and addressed each of these issues and removed all unused SFT reading functions.

Detailed changes

  1. All of the commented-out code was removed
  2. Removed unused user-input arguments (specifically freqRes, psrInput, psrEphemeris, earthFile, and sunFile)
  3. Renamed user-input argument blocksRngMed to blocksRngMean since the median calculation was not used, just mean
  4. Replaced ancient SFT reading functions (copies of code from SFTfileIO.c, including methods to read v1 SFTs) with simple function-call to XLALSFTCatalogTimeslice() and XLALLoadSFTs() since we want to be loading only 1 full-band SFT at a time
  5. Stopped saving the 3 extra, unused 0-byte files
  6. Use XLAL vector-types, replacing some allocated arrays
  7. Free-memory properly at the end of the program execution

Tests

Original

$ lalapps_spec_avg_long --version
Starting spec_avg...
LAL: 7.0.0.1 (CLEAN f253e1307b9c19b0fa974fe627651db483f38170)
LALPulsar: 2.0.0.1 (CLEAN f253e1307b9c19b0fa974fe627651db483f38170)
LALApps: 6.26.1.1 (CLEAN f253e1307b9c19b0fa974fe627651db483f38170)
$ lalapps_spec_avg_long -h
Starting spec_avg...

Usage: lalapps_spec_avg_long [-h|--help] [-v|--version] [@<config-file>] -p|--SFTs -I|--IFO -s|--startGPS -e|--endGPS -f|--fMin -F|--fMax [-w|--blocksRngMed] [-o|--outputBname] -r|--freqRes -t|--timeBaseline [-P|--psrInput] [-S|--psrEphemeris] [-y|--earthFile] [-z|--sunFile]

$ lalapps_spec_avg_long --SFTs="./*.sft" -I H1 -s 0 -e 2000000000 -f 10 -F 20 -o old_version -t 1800 -r 0.0001
Starting spec_avg...
Calling LALSFTdataFind with SFTpatt=./*.sft
Now have SFT catalog with 2 catalog files
Looping over SFTs to compute average spectra
Extracting SFT 0...
Extracting SFT 0...
firstbin=18000, lastSFTbin=36000 
lastBin2read = 36000
numBins=18001, f0=10.000000, deltaF=0.000556
Extracting SFT 1...
Extracting SFT 1...
firstbin=18000, lastSFTbin=36000 
lastBin2read = 36000
About to do calculation of averages...
Sample: timeavg[0]=8.23042e-42, timeavgwt[0]=1.12923, sumweight[0]=2.40116e+41
Destroying Variables
Done Destroying Variables
Closing Files
end of spec_avg
Spec_avg_done!

Updated version with this patch

$ lalapps_spec_avg_long --version
LAL: 7.0.0.1 (CLEAN 3fd79bba152a2c04bae26fe2fa356bc901206309)
LALPulsar: 2.0.0.1 (CLEAN 3fd79bba152a2c04bae26fe2fa356bc901206309)
LALApps: 6.26.1.1 (CLEAN 3fd79bba152a2c04bae26fe2fa356bc901206309)
$ lalapps_spec_avg_long -h

Usage: lalapps_spec_avg_long [-h|--help] [-v|--version] [@<config-file>] -p|--SFTs -I|--IFO -s|--startGPS -e|--endGPS -f|--fMin -F|--fMax [-w|--blocksRngMean] [-o|--outputBname] -t|--timeBaseline

$ lalapps_spec_avg_long --SFTs="./*.sft" -I H1 -s 0 -e 2000000000 -f 10 -F 20 -o new_version -t 1800
Starting spec_avg_long...
Calling XLALSFTdataFind with SFTpatt=./*.sft
Now have SFT catalog with 2 catalog files
Looping over SFTs to compute average spectra
Extracting SFT 0...
numBins=18001, f0=10.000000, deltaF=0.000556
Extracting SFT 1...
About to do calculation of averages...
Sample: timeavg[0]=8.23042e-42, timeavgwt[0]=1.12923, sumweight[0]=2.40116e+41
Destroying Variables
Done Destroying Variables
Closing Files
end of spec_avg
Spec_avg_done!

Comparison

$ diff old_version.txt new_version.txt
$
$ diff old_version_PWA.txt new_version_PWA.txt
$

As evidenced by the comparison tests, the old and new versions have identical results.

API Changes and Justification

I'm not sure which category it should fall into when removing of completely unused user-input arguments. Also, one argument was renamed ("med" ==> "mean") which modifies the existing API. Do I check both modifies existing and removes existing?

Backwards Compatible Changes

  • This change introduces no API changes
  • This change adds new API calls

Backwards Incompatible Changes

  • This change modifies an existing API
  • This change removes an existing API

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

This update touches almost every line of the code, but I verified the results are identical on a set of two simulated SFTs. Natural candidates for review are @karl-wette, @david-keitel, or Keith Riles.

Edited by Karl Wette

Merge request reports