hera_sim and pyuvdata API changes have broken things.

[aewallwi]

API changes to hera_sim and pyuvdata are breaking hera_pspec in various places. A few lines that are using outdated API calls are

hera_pspec/hera_pspec/testing.py

Line 488 in 13e89d6

J2K = hs.noise.jy2T(freqs/1e9, OmegaP[key[2]]) / 1e3

hera_pspec/hera_pspec/testing.py

Line 508 in 13e89d6

n = hs.noise.sky_noise_jy(Tsys, freqs/1e9, lsts, OmegaP[bl[2]], inttime=int_time)

and

hera_pspec/hera_pspec/pspecdata.py

Line 213 in 13e89d6

uvutils.uvcalibrate(dset, cal, inplace=True, prop_flags=cal_flag, flag_missing=cal_flag)

hera_pspec/hera_pspec/pspecdata.py

Line 216 in 13e89d6

uvutils.uvcalibrate(dset_std, cal, inplace=True, prop_flags=cal_flag, flag_missing=cal_flag)

There may be more.

[steven-murray]

@aewallwi these look like easy enough fixes. I'll get to it sometime this week.

[adeliegorce]

Hi, do you have any update on this?
I managed for tests to run without errors yesterday by requesting hera_pspec to use a given hera_sim commit (HERA-Team/hera_sim@2005a45) as suggested by @piyanatk but 24hrs later they are now failing because of the function changes mentioned above by @aewallwi. Any idea of what changed? @steven-murray have you been able to fix the pyuvdata version clash?
Thanks!

[aewallwi]

@adeliegorce, are the problems in pyuvdata or are the hera_sim calls still causing problems?

[adeliegorce]

No, since I call a previous version of hera_sim which did not require pyuvadata<2.2.0, this part is fine, but now I get the same errors as you did in your first post, for example hs.noise.jy2T does not exist. The weird part is that all tests were running fine yesterday afternoon Eastern time

[steven-murray]

@adeliegorce if you're using an old version of hera_sim, it should have the jy2T function. It sounds like you're maybe using the wrong env, or have inadvertantly updated hera_sim since yesterday?

BTW I think current hera_sim does not required pyuvdata<2.2.0?

[steven-murray]

The hera_sim part of this issue is resolved, right? Is this issue still open because of the other calls?

[adeliegorce]

I don't think the hera_sim issue is fixed because in the tests, hera_pspec installs it itself by calling the main git repo and then there is a version clash on pyuvdata.
The tests that fail are the ones ran by Git within the PR so normally the problem should not come from my local environment. I've been trying to re-install both packages from scratch, at different versions, several times and there is always either an issue on pyuvadata or on the hera_sim functions...

[aewallwi]

It looks like in this run

https://github.com/HERA-Team/hera_pspec/runs/3135559237?check_suite_focus=true

the three failures are all coming from the out of date pyuvdata API.

[steven-murray]

@adeliegorce can you post a link to the tests you are trying to run but are failing? I'm not quite sure I'm following. The commit you are using does NOT have jy2T function in it -- you need to use the updated API from hera_sim.

I think @r-pascua understands the pyuvdata < 2.2.0 requirement more than me. I was under the impression that it's not required for most things.

Sorry I'm not being more help!

[adeliegorce]

Okay, there might be a workaround then...
Here are the test runs that are failing in the PR: https://github.com/HERA-Team/hera_pspec/actions/runs/1056673783. We only added a notebook for the PR so there is no reason the code should break...
Thanks anyway for taking the time @steven-murray!

[adeliegorce]

And that is the ones that were running yesterday although I just changed the notebook from one commit to the other https://github.com/HERA-Team/hera_pspec/actions/runs/1053030265

[steven-murray]

It looks like the one that's failing is installing pyuvdata 2.2.1 while the one that passed is installing pyuvdata 2.1.5. So it looks like it's still a problem with API calls to the updated pyuvdata.

[r-pascua]

After browsing the logs, it looks like the failures are related to changes made to pyuvdata.uvutils.uvcalibrate, as @aewallwi pointed out initially. This is causing problems in two places: one place is the test_construct_pstokes_multipol function, which is traced back to the antenna names not being set in the calibration file; the other is in the PSpecData.add method, which traces back to the flag_missing parameter being removed once the API changed so that missing antennas names raise an error if the (new) ant_check parameter is not set to False.

I'm not sure if there's a neat way to address this problem in the code, since it seems like the changes made in pyuvdata are in line with the notion that it's the responsibility of the user to ensure that the files/objects being used are complete. Additionally, the change to the call signature (as well as how the uvcalibrate function checks which antennas are missing) means that there isn't a neat solution that simultaneously handles all versions of pyuvdata—it would need to change the parameters it sets in the call to uvcalibrate based on the version of pyuvdata being used, unless a restriction was placed on which versions of pyuvdata are supported for use with hera_pspec.

As for dealing with the code, I see two solutions:

1. Adjust the call to uvcalibrate based on which version of pyuvdata is being used, with the understanding that this may cause some unexpected behavior (e.g. if the updated version set ant_check=False, then all the data would be flagged in (at least) the test_construct_pstokes_multipol function, which I assume is due to none of the antennas being named in the calibration file;
2. Restrict the version of pyuvdata in both the setup configuration and the testing configuration files. In order to maintain compatibility with the current version of hera_sim (pending changes to bda which may or may not have been made yet), this would need to be <2.2.0.

An alternative solution is to follow the idea that it's the responsibility of the user to make sure the files/objects they're using have the correct (meta)data, and to just remove the flag_missing=cal_flag option from the call to uvcalibrate where appropriate. If this route is chosen, then the calibration file used for tests will need to be updated so that the antenna names are filled out and match those in the test data.

There may be other ways to deal with this, but this is about all I can think of based on my understanding of what's breaking in hera_pspec and what's changed in pyuvdata. 🤷

[adeliegorce]

Thanks for your diagnostis @r-pascua! Maybe we should discuss this at the next hera_pspec meeting so that everyone is aware of the problem/solution.

[r-pascua]

Here is a quick summary of what we decided on the telecon. We think that the way forward is to be performed in two steps:

1. Hard-code the noise realization in the relevant test file and remove the hera_sim dependency;
2. Update hera_pspec to work only for pyuvdata>=2.2.0 and update the call to the relevant function that has changed (uvcalibrate).

I think we also want input from @philbull and other interested parties.

[bhazelton]

I apologize for the pyuvdata changes in uvcalibrate (actually I apologize that the initial version was released as it was, it never should have been allowed to apply calibrations to data with mismatching metadata). The underlying problem is that hera_cal used to write calfits files with incorrect metadata, but I believe that has been fixed, so if you update to newer files it shouldn't be a problem. If you really want to stick with the older files for some reason, I'd be happy to help you figure out how to correct the metadata in the UVCal objects immediately after they are read from the files in your tests.

[philbull]

Hi folks, I've issued PR #346 to fix all this stuff. It removes the dependency on hera_sim, and should fix the calfits files too.

[steven-murray]

@philbull and @aewallwi is this now addressed?