Phase 3 Funding: Improving Compressed Image performance and Dask integration in io.fits. - #275

Because the estimated total of all the proposals is a bit higher than the amount available for allocation, we suggest that you (@Cadair ) provide a budget range - i.e., a "minimum for this project to be viable" and "maximum you could use effectively for this project".

@martindurant This is still a very early draft, but do you have any thoughts or recommendations on how you would go about adding dask support to reading FITS files in Astropy?

Yes, I will certainly have thoughts!

In the meantime, you might be interested in this codec which can read variable-length table data from FITS, i.e., the heap, which would be a necessary prerequisite to loading the same data in parallel via zarr rather than astropy.io.fits. (The "rice" compressor isn't implemented in numcodecs, though, or - as far as I can tell - anywhere at all)

Some thoughts:

• it is very reasonable to want to de decompression in parallel with dask, if you can release the GIL and overheads are reasonable. When we are comparing runtimes of ~100ms, overheads will be important, but up to 4-8x speedup might still be possible with the threaded scheduler
• more important is lazy/out-of-core or selective fetching of the data, both to cutdown on transfer bandwidth (I am thinking of remote loading) as well as improving CPU load
• probably remote loading can only work with a python implementation, being able to pass a file-like rather than a real local file handle, so I am fully behind this
• the structure of a single compressed image is perfectly aligned with a kerchunk representation, loading piecewise via a single decompression codec. So writing a references scanner for kerchunk would be be pretty simple if we can handle the compression codecs, and you would get remote/parallel/out-of-core fo free. This turns the problem on its head by using zarr as the IO layer instead of fits, but I would suggest astropy should have zarr as a supported data format backend. It would also be possible, but more work, to bundle a smart IO layer under fits.HDU (see, e.g., what fsspec.parquet does to preload the bytes ranges we know we will need, and then pass this prefilled file instance to the loader).

@martindurant Just hypothetically, what would astropy need to expose for kerchunk to be able to "just" call astropy to expose a zarr array? @astrofrog and I were discussing different approaches to this problem today, and I think we were discussing implementing something fundamentally similar to how kerchunk has implemented FITS support but providing an API more familiar to current astropy.io.fits users. Obviously, if we were to end up taking that path, it would be redundant for both of us to carry around similar implementations, so knowing how to combine them would be great.

I think having astropy be able to read from zarr should be very simple, and kerchunk/dask would come for free. Whether it would want to export the same hdu/header interface as fits is another matter. Should check up on what asdf does right now.

Did someone say asdf ? cc @WilliamJamieson

Please react to this comment to vote on this proposal (👍, 👎, or no reaction for +0).

Congratulations! The community has expressed support for this FR, and the Finance Committee has determined it can be funded. This FR has been approved for $22,500, to be funded from the NASA grant.

@aaryapatil is the Finance Committee member who will be supporting this project. They will update this PR with the amount and then merge it, as well as being in contact with you to set up any necessary agreements and relevant Open Collective financial arrangements. They will also open a new issue where progress on this project can be discussed.

Why is this not given the full amount, while proposals with less support got the full request?

It is disappointing that the second half hasn't been funded (as that's the "flashy" part), but the first half is still required ground work, for both the second half of this proposal and for kerchunk to support tile compression etc.

@Cadair - I think this was a mis-understanding of the proposal amount! That is, I think the intent was to fund this whole proposal, not just the first part, and it was mis-understood as the 150 hours being the requirement for both parts. Will confirm that to be sure but my recollection is that we intended to fully fund this proposal.

(In #298 there's already a clarification for the next cycle to make sure the budget prompt clearly says the total to prevent such misunderstandings in the future.)

@eteq - ok thanks for clarifying! Just to check, does this mean that the funded amount for this proposal will be updated, or is it too late and it will be funded only for the first 150 hours?

@astrofrog @Cadair - confirming this after some discussion with the finance committee. Indeed, there was a misunderstanding about your intended budget. The intent was to approve the whole FR, but in budgeting there was a misunderstanding that you were only asking for the first phase in this cycle.

So here's the solution we propose: we are approving this whole FR since it has lots of community support (including me 😉). But we are releasing only the first phase now. Once that phase is done (or getting close to done), you can check in with the finance committee (i.e. @aaryapatil as your prime contact for this FR), and we will hopefully release the second phase at that time.

The main reason for doing this is because the second phase exceeds our nominal budget for the year given that we budgeted only for the first phase. Based on last cycle, this should be no problem because some folks needed extensions and therefore some of the money rolled over to the following cycle, leaving some slack in the budget. So when we check in for the second phase of this FR, some time will have passed in the cycle so then we can either say "looks like we have slack in the budget like last time, go ahead and spend it", or the much less likely scenario of "everyone's spending money faster than expected, you may need to wait until NASA releases the next chunk of budget."

Does that sound like an ok plan to you?

Ok @eteq that sounds great. (There's a non-zero chance we find ourselves with less time anyway 🙈 )

@aaryapatil - we have now completed the first bullet point in part 1 of the proposal which was to remove (as much as possible) CFITSIO from astropy:

• A new tiled image compression submodule mostly replacing cfitsio astropy#14252
• Remove ability to link against external installation of CFITSIO astropy#14311

These have both been merged (in addition to simplifying/removing a lot of FITS-related code, this works also speeds up compilation of the core package by 40% or so, so it has had benefits beyond just FITS).

We ended up using 138h for this work (invoiced today) instead of the original 100h we budgeted, so we now only have 12h left for the remainder of part 1. We are wondering if the finance committee would be happy to approve the additional 100h of funding we originally requested, as this would allow us to complete part 1 and also do a big fraction of part 2? (see #275 (comment) for more details)

Are you sure you mean astropy/astropy#1425 ? That was merged in 2013... Probably a typo of the PR number?

Oops deleted a character by mistake when reformatting my comment 😅 - fixed now

This looks great. Thanks, @astrofrog! Could you please post your update on the issue (#316) instead of on the PR. We aim to use the issue for keeping track of the progress. We will also get back to you about the the additional 100h of funding (hopefully next week).