Read calfits files without memory-mapping

[tyler-a-cox]

Description

UVCal.read_calfits now opens calfits files with memmap=False. The calfits reader always reads the data in full (select happens after the read) or, with read_data=False, doesn't read it at all, so memory-mapping doesn't buy anything here. Metadata-only reads still only read the headers and the antenna table.

The gain-type quality_array is copied out of the FITS data array so the rest of that array can be freed once the gains and flags are built.

I added a test (test_read_not_memmapped in tests/uvcal/test_calfits.py) that checks the arrays aren't memory-mapped after a read and that the gain quality array doesn't hold onto the FITS data. I also added a CHANGELOG entry.

Motivation and Context

For HERA phase II we read about 1850 calfits files per night off of NRAO's Lustre file system during calibration smoothing, and a lot of that time is spent in memory-mapped reads. On a network file system, memory-mapping pulls in the file a piece at a time as the arrays are accessed, and each of those pieces has to wait on the network. Reading without memory-mapping grabs the data in one big read.

I timed read_calfits on some of our calfits files on Lustre, clearing the page cache first. The median read took 2.1 s per file with memory-mapping and 0.23 s per file without it, so about 9x faster. Peak memory was the same either way (about 60 MB per read), and the arrays were identical. On local disk the two were about the same.

This follows #1651, which stopped read_uvfits from memory-mapping full reads.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation change (documentation changes only)
• Version change
• Build or continuous integration change
• Other

Checklist:

• I have read the [contribution guide](https://github.com/RadioAstronomySoftwareGroup/pyuvdata/blob/main/.github/CONTRIBUTING.md).
• My code follows the code style of this project.

Other:

• I have updated any docstrings associated with my change using the [numpy docstring format](https://numpydoc.readthedocs.io/en/latest/format.html).
• I have updated the readme and/or tutorial to reflect my changes (if appropriate).
• I have added/updated tests to cover my changes (if appropriate).
• I have updated the [CHANGELOG](https://github.com/RadioAstronomySoftwareGroup/pyuvdata/blob/main/CHANGELOG.md).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (63203e5) to head (474308f).

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #1724   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           71        71           
  Lines        23912     23912           
=========================================
  Hits         23912     23912           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bhazelton]

@tyler-a-cox thank you for this!

My reading of the astropy docs is that setting memmap=True is what lets you read in part of the data. So I think we should only use memmap=True when we're only reading in a portion of the data. I thought that's what we did on uvfits, but looking at it now it seems like memmap is always False, so there may be a bug there.

I'm not sure we need a read parameter for this if we set it sensibly based on whether or not there's a select, but I could be convinced otherwise with a good use case.

[tyler-a-cox]

@bhazelton thanks for giving this a look over! I'm definitely open to removing memmap as a argument and only setting it to True when there is a partial read. I'll make some changes and ping when that's done.

[tyler-a-cox]

@bhazelton after reviewing the code, I realized that calfits never actually does partial reads. read_calfits doesn't take any select arguments, and UVCal.read reads the whole file and then calls select afterwards. So I don't think there is a case where memmap=True would help right now.

I can either implement select on read for calfits, which feels like a pretty large structural change, or just always open calfits files with memmap=False and remove the argument. Do you have a preference?

[bhazelton]

Ah, I had forgotten that we hadn't implemented partial read on cal fits. Let's just set to always be False.

[tyler-a-cox]

Okay, looks like that simplified things a little. Should be ready to look at now!

[bhazelton]

Looks great, thank you @tyler-a-cox !