RadioAstronomySoftwareGroup / pyuvsim

A ultra-high precision package for simulating radio interferometers in python on compute clusters.
https://pyuvsim.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
43 stars 7 forks source link

initialize_uvdata_from_params return uvdata with channel_width=1.0 when Nfreqs is 1 #380

Closed piyanatk closed 1 year ago

piyanatk commented 2 years ago

initialize_uvdata_from_params will always return a UVData object with channel_width=1.0 when the configuration file has Nfreqs set to 1 regardless of the actual channel_width.

bhazelton commented 2 years ago

@piyanatk

I tried to replicate your problem in a test, but it passed:

def test_freq_parser_1freq_channel_width():
    input_freq_dict = {
        "Nfreqs": 1,
        "channel_width": 50e3,
        "start_freq": 150e6,
    }
    output_freq_dict = pyuvsim.parse_frequency_params(input_freq_dict)

    expected_freq_dict = {
        "Nfreqs": 1,
        "freq_array": [150e6],
        "channel_width": [50e3],
        "Nspws": 1,
    }

    assert output_freq_dict == expected_freq_dict

Is this similar to what you're setting? What version are you running?

piyanatk commented 2 years ago

@bhazelton I checked again, and it looks like the issue is actually _complete_uvdata replacing the channel_width attribute. This was done in hera_sim to prepare the uvdata object for visibility simulation. https://github.com/HERA-Team/hera_sim/blob/9576e78bd8807473c40152f9b6b61745ca9dc879/hera_sim/visibilities/simulators.py#L168-L184

bhazelton commented 2 years ago

@piyanatk Are you sure you're specifying the channel width in your parameter file?

I expanded my test to include initialize_uvdata_from_params and _complete_uvdata and I'm still getting that the channel_width is what I set it to, not 1.

I'll note that there is no logic related to channel_width in either initialize_uvdata_from_params or _complete_uvdata, so I didn't think they would be related but I checked them anyway.

def test_freq_parser_1freq_channel_width():
    input_freq_dict = {
        "Nfreqs": 1,
        "channel_width": 50e3,
        "start_freq": 150e6,
    }
    output_freq_dict = pyuvsim.parse_frequency_params(input_freq_dict)

    expected_freq_dict = {
        "Nfreqs": 1,
        "freq_array": [150e6],
        "channel_width": [50e3],
        "Nspws": 1,
    }

    assert output_freq_dict == expected_freq_dict

    # Now make a UVData object to check its channel_width
    param_filename = os.path.join(SIM_DATA_PATH, "test_config", "param_10time_10chan_0.yaml")
    param_dict = pyuvsim.simsetup._config_str_to_dict(param_filename)

    freq_keywords = [
        'freq_array', 'start_freq', 'end_freq', 'Nfreqs', 'channel_width', 'bandwidth'
    ]
    for key in freq_keywords:
        param_dict["freq"].pop(key, None)

    for key, value in input_freq_dict.items():
        param_dict["freq"][key] = value

    assert param_dict["freq"]["channel_width"] == input_freq_dict["channel_width"]
    uvd, _, _ = pyuvsim.initialize_uvdata_from_params(param_dict)

    assert uvd.channel_width == np.asarray([input_freq_dict["channel_width"]])
    assert uvd.freq_array == expected_freq_dict["freq_array"]

    uvd2 = pyuvsim.simsetup._complete_uvdata(uvd)

    assert uvd2.channel_width == np.asarray([input_freq_dict["channel_width"]])
    assert uvd2.freq_array == expected_freq_dict["freq_array"]
piyanatk commented 2 years ago

@bhazelton Yes, I am very sure. My test was not very sophisticated, but please see the attached screen shot for what I did.

Screen Shot 2022-02-15 at 19 51 31
bhazelton commented 2 years ago

@piyanatk Thanks, that was helpful. I saw that you were using v1.2.1 (as opposed to the latest main branch), so I tested on that version and I see the error. So I think that if you get the latest main it will fix it, but we should also make a new release.

piyanatk commented 2 years ago

@bhazelton, using the main branch indeed fixed the problem. Thank you!

piyanatk commented 1 year ago

Fixed in recent version