Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CASA-based MS calibration tables #1391

Merged
merged 45 commits into from
Apr 18, 2024
Merged

Conversation

kartographer
Copy link
Contributor

@kartographer kartographer commented Feb 5, 2024

Adds the MSCal class, and other related features to UVCal.

Description

NOTE: This PR is an early draft and still WIP -- internal consistency within pyuvdata has been checked, but CASA testing has been much more limited. Comments/feedback are very much welcome!

At this point, the PR has advanced out of draft stage, and is ready for "full" review (though I think I've already gotten a good number of helpful comments thusfar). So far the code has been tested against SMA and LWA data, and I think is ready for a sort of "beta" release. There's a couple of outstanding items to take care of, which are on the checklist below (and will be ticked off upon completion accordingly). Comments/feedback still very much welcome.


The MSCal class -- a subclass of UVCal -- has been added, which enables read/write support for MS calibration tables with sky-based calibration schemes (gains, bandpass, delays). To support full loopback of the information with the MS tables, handling of phase center information (and the associated parameters Nphase, phase_center_catalog, and phase_center_id_array) has been added, along with the ability to create "flex-Jones" objects, which mimics the UVData flex-pol functionality (support for which required the addition of the optional parameter flex_jones_array).
Additional optional parameters also include scan_number_array and antenna_diameters -- all of the above mimicking their UVData counterparts.

One additional parameter has been added -- ref_antenna_array -- which allows for the reference antenna to vary across the time-axis of a UVCal object. The MS format supports such variation, and given that there's no reason one has to keep the same reference antenna across a gains solution, I decided to add support for it to provide better preservation of information when reading/writing MS gains table.

SMA-based MS calibration files have also been added for testing, which will also eventually got folded in the relevant tutorial. Some additional clean-up has also been done in the data folder of the repository to reduce the overall footprint for packaging (tarball was ~65 MB, now appears to be ~45 MB from tests on my laptop).

Separately, a bug affecting UVBase.__eq__ has been fixed, where allowed_failures was being ignored if a parameter was required (i.e., non-optional). An additional bug which affected how select, __add__, and fast_concat dealt with identifying desired or overlapping frequency channels, which only affected UVCal objects where flex_spw=True that had more than one spectral window.

Motivation and Context

Adds support for reading/writing UVCal objects into Measurement Set format, so that they may be used within/read from various CASA routines.

Closes #143
Closes #1016
Closes #1202
Closes #1407

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

Bug fix checklist:

  • My fix includes a new test that breaks as a result of the bug (if possible).
  • All new and existing tests pass.
  • I have updated the CHANGELOG.

New feature checklist:

  • I have added or updated the docstrings associated with my feature using the numpy docstring format.
  • I have updated the tutorial to highlight my new feature (if appropriate).
  • I have added tests to cover my new feature.
  • All new and existing tests pass.
  • I have updated the CHANGELOG.

Copy link

codecov bot commented Feb 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.93%. Comparing base (8355244) to head (332efce).

Additional details and impacted files
@@             Coverage Diff             @@
##           prep_v3.0    #1391    +/-   ##
===========================================
  Coverage      99.92%   99.93%            
===========================================
  Files             39       41     +2     
  Lines          21488    22390   +902     
===========================================
+ Hits           21472    22375   +903     
+ Misses            16       15     -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@bhazelton bhazelton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is still a draft, but I took a look and had a few comments.

pyuvdata/uvcal/ms_cal.py Outdated Show resolved Hide resolved
pyuvdata/uvcal/ms_cal.py Outdated Show resolved Hide resolved
pyuvdata/uvcal/ms_cal.py Outdated Show resolved Hide resolved
pyuvdata/uvcal/ms_cal.py Outdated Show resolved Hide resolved
pyuvdata/uvcal/ms_cal.py Outdated Show resolved Hide resolved
pyuvdata/uvcal/ms_cal.py Outdated Show resolved Hide resolved
@rlbyrne
Copy link
Contributor

rlbyrne commented Mar 3, 2024

I'm testing this branch with some calibration solutions I'm working with. I have a CASA-generated *.bcal file and a pyuvdata-generated *.calfits file.

  1. Reading the *.bcal file with cal_orig.read_ms_file(path/to/file.bcal) worked, and cal_orig.check() passes.
  2. I read the *.calfits file with cal.read_calfits(path/to/file.calfits), and cal.check() passes.
  3. I tried writing the cal object with cal.write_ms_cal(path) and got this error:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/pyuvdata/uvcal/uvcal.py", line 6440, in write_ms_cal
    ms_cal_obj.write_ms_cal(filename, **kwargs)
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/pyuvdata/uvcal/ms_cal.py", line 410, in write_ms_cal
    time_array = (Time(time_array, format="jd", scale="utc").mjd * 86400.0)[
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/astropy/time/core.py", line 1580, in __init__
    self._init_from_vals(val, val2, format, scale, copy,
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/astropy/time/core.py", line 433, in _init_from_vals
    self._time = self._get_time_fmt(val, val2, format, scale,
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/astropy/time/core.py", line 499, in _get_time_fmt
    raise ValueError(
ValueError: Input values did not match the format class jd:
TypeError: for jd class, input should be doubles, string, or Decimal, and second values are only allowed for doubles.
  1. I set cal.time_array = np.array([np.mean(cal_orig.time_array)]) and cal.lst_array = np.array([np.mean(cal_orig.lst_array)]) (.calfits file only has one time step) and tried rerunning cal.write_ms_cal(path) and got this error:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/pyuvdata/uvcal/uvcal.py", line 6440, in write_ms_cal
    ms_cal_obj.write_ms_cal(filename, **kwargs)
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/pyuvdata/uvcal/ms_cal.py", line 446, in write_ms_cal
    self.antenna_names.index(self.ref_antenna_name)
AttributeError: 'numpy.ndarray' object has no attribute 'index'
  1. I set cal.antenna_names = list(cal.antenna_names) and confirmed that cal.check() succeeds. I then reran cal.write_ms_cal(path) and got this error:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/pyuvdata/uvcal/uvcal.py", line 6440, in write_ms_cal
    ms_cal_obj.write_ms_cal(filename, **kwargs)
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/pyuvdata/uvcal/ms_cal.py", line 532, in write_ms_cal
    subarr = np.transpose(subarr, [2, 0, 1, 3])
  File "<__array_function__ internals>", line 200, in transpose
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/numpy/core/fromnumeric.py", line 668, in transpose
    return _wrapfunc(a, 'transpose', axes)
  File "/opt/devel/rbyrne/envs/pyuvdata/lib/python3.8/site-packages/numpy/core/fromnumeric.py", line 57, in _wrapfunc
    return bound(*args, **kwds)
ValueError: axes don't match array

Let me know if you want the files I used for this test.

@kartographer
Copy link
Contributor Author

Thanks @rlbyrne -- yes, if you could send along the files, it'd be helpful for trying to chase down the issues you were running into.

@rlbyrne
Copy link
Contributor

rlbyrne commented Mar 3, 2024

Thanks @rlbyrne -- yes, if you could send along the files, it'd be helpful for trying to chase down the issues you were running into.

Just sent them via Slack

@kartographer
Copy link
Contributor Author

@rlbyrne -- looks like that final issue are running into is because I'm assuming future array shapes without explicitly checking. I'll fix that in the code, but I think if you run the UVCal.use_future_array_shapes method it should hopefully work.

@kartographer
Copy link
Contributor Author

kartographer commented Mar 19, 2024

Recording this here for the sake of documentation -- I've at least had one successful full test of exporting SMA-based solutions into CASA and applying them via applycal. Both phases and amplitudes look as expected and are in agreement with what our internal pipeline has generated. There seems to be a slightly unexpected "extra" conjugation required for the gains (that effectively cancels out that normally seen reading in an MS file, and that I can reasonable understand without too much tortured logic), although it may be helpful to verify that the same behavior is seen in delay solns as well. Given this successful test, this PR is looking like it's ready to advance out of draft status...

Screenshot 2024-03-19 at 06 57 42
Screenshot 2024-03-19 at 06 57 22

@rlbyrne
Copy link
Contributor

rlbyrne commented Mar 22, 2024

@rlbyrne -- looks like that final issue are running into is because I'm assuming future array shapes without explicitly checking. I'll fix that in the code, but I think if you run the UVCal.use_future_array_shapes method it should hopefully work.

@kartographer I'm just getting back to this after travel... I used write_ms_cal after running use_future_array_shapes . The function did produce a *.bcal file, but it looks like there's a bug in the path parsing. In addition to the intended directory (cal46_newcal_rewritten.bcal), it made 5 other directories in the same location called cal46_newcal_rewritten.bcal::ANTENNA, cal46_newcal_rewritten.bcal::FIELD, cal46_newcal_rewritten.bcal::HISTORY, cal46_newcal_rewritten.bcal::OBSERVATION, and cal46_newcal_rewritten.bcal::SPECTRAL_WINDOW. I assume the "::" should just be "/" and that would fix this?

@kartographer
Copy link
Contributor Author

@kartographer I'm just getting back to this after travel... I used write_ms_cal after running use_future_array_shapes . The function did produce a *.bcal file, but it looks like there's a bug in the path parsing. In addition to the intended directory (cal46_newcal_rewritten.bcal), it made 5 other directories in the same location called cal46_newcal_rewritten.bcal::ANTENNA, cal46_newcal_rewritten.bcal::FIELD, cal46_newcal_rewritten.bcal::HISTORY, cal46_newcal_rewritten.bcal::OBSERVATION, and cal46_newcal_rewritten.bcal::SPECTRAL_WINDOW. I assume the "::" should just be "/" and that would fix this?

Hey @rlbyrne -- I do remember seeing this error at some point, but I believe I've fixed in the last couple of weeks. Trying with both files you sent (which now should alert you when you aren't using future array shapes), I seem to get sensible looking files. Could I ask you to delete/re-pull the branch and try again to see if you see the same behavior?

@rlbyrne
Copy link
Contributor

rlbyrne commented Mar 25, 2024

@kartographer I'm still getting the error. I'm on branch casa_gains and it is up to date.

@kartographer
Copy link
Contributor Author

@rlbyrne -- can you check what version of casacore you have installed? Just want to rule out an external API issue...

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@bhazelton bhazelton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, Thank you!!

@bhazelton bhazelton merged commit eb3e8b8 into prep_v3.0 Apr 18, 2024
41 of 44 checks passed
@bhazelton bhazelton deleted the casa_gains branch April 18, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Calibration SMA Issues related to handling of SMA data
Projects
None yet
3 participants