Add extract autos script #393
Conversation
|
I have not yet tested that this script actually runs, but I've done parts of it in a jupyter notebook. I will update this PR when I have actually checked the script. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #393 +/- ##
=======================================
Coverage 95.99% 95.99%
=======================================
Files 17 17
Lines 6113 6124 +11
=======================================
+ Hits 5868 5879 +11
Misses 245 245
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This is looking good! I left a few comments, but the only thing I feel really strongly about is the silent passing in lines 38-39, which I think needs to have a more explicit warning (or just a straight up failure).
On a different note, I feel like hera_pspec is the wrong place for this script. I would argue that this should maybe go somewhere in hera_pipelines, for lack of a better place to put it. Curious to hear thoughts from other people, though.
| "certainly incorrect.") | ||
|
|
||
| outfile = f"zen.LST.0.00000.{args.sumdiff}.{args.label}.foreground_filled.xtalk_filtered.chunked.waterfall.autos.uvh5" | ||
| main_uvd.write_uvh5(outfile, clobber=True) |
There was a problem hiding this comment.
I think for best practice, clobber should be an argument passed to the script.
| found_autos = False | ||
| for file_ind, file in enumerate(args.flist): | ||
| check_for_sumdiff(file) | ||
| try: | ||
| main_uvd = UVData() | ||
| main_uvd.read(file, ant_str="auto") | ||
| found_autos = True | ||
| break | ||
| except ValueError: # There were no autos in that file | ||
| continue |
There was a problem hiding this comment.
I found this block of code a little difficult to parse at first--it took me a few reads to wrap my head around it. I would maybe instead add a comment to the effect of "Find the first file that has autos."
| start_ind = file_ind + 1 | ||
| if start_ind < len(args.flist): | ||
| for file in args.flist[file_ind + 1:]: |
There was a problem hiding this comment.
I'm not sure how important it is to track whether only one file had autocorrelations in it, but if it's not that important to do, then you can replace this block with:
for file in args.flist[file_ind + 1:]
If file_ind + 1 is greater than len(args.flist), this just doesn't do any iterations.
| except ValueError: | ||
| continue |
There was a problem hiding this comment.
This worries me. Are you certain the only way a ValueError can be raised is by a file not having autos in it? What if the autos in one file differ from the autos in a different file? I'd think that pyuvdata would also have an issue with that. At any rate, I think something like warnings.warn("Some files missing autos") should be emitted before continuing.
| try: | ||
| new_uvd = UVData() | ||
| new_uvd.read(file, ant_str="auto") | ||
| main_uvd.__add__(new_uvd, inplace=True) |
There was a problem hiding this comment.
I'd suggest adding an axis argument to the parser, which defaults to None, that will let you do fast concatenation here by specifying axis=args.axis. Our typical use case should let us use axis='blt' to speed up the concatenation.
|
Closing since this all lives in |
I'm not sure if this script belongs in pspec (maybe it belongs in cal) but basically in the face of fringe rate filtering it will be a huge speedup to have the autos extracted to their own waterfall file after LST binning for the sake of noise error propagation and any other post-LST-binning analysis that might be fun to do on the autos.
I've added a script that does the extraction with basic UVData operations. I also added an argument parser for this script in utils, with associated unit test.
The script has some very slim checking to see if the supplied files are not catastrophically different than what's expected, but it's generally relying on the pipeline module doing the right thing.