chime-experiment / ch_util

CHIME utilities
https://chime-experiment.github.io/ch_util
MIT License
3 stars 3 forks source link

[WIP] Support distributed read for other acquisition files #30

Open tristpinsm opened 2 years ago

tristpinsm commented 2 years ago

I made these changes in order to be able to read CalibrationGainData files into distributed containers.

tristpinsm commented 2 years ago

the other part of the unicode index map fix is in radiocosmology/caput#201

jrs65 commented 2 years ago

Ah interesting. I was facing similar issues with the HFB code. In the end I opted to go a slightly different route and write a draco style container that could read the archived HFB code (chime-experiment/ch_pipeline#99). I figured I could probably do the same for many of the other formats to (e.g. CorrData).

I'll have a look at this later. It is a massive pain to need to be maintaining multiple implementations for distributed IO, so we should see if there are reasons to keep it separate, and if we can combine any of it together.

tristpinsm commented 2 years ago

Ah interesting. I was facing similar issues with the HFB code. In the end I opted to go a slightly different route and write a draco style container that could read the archived HFB code (chime-experiment/ch_pipeline#99). I figured I could probably do the same for many of the other formats to (e.g. CorrData).

Yeah I thought about taking this approach, but decided I didn't want to tackle changing the whole framework, especially because the CorrData code is so complicated. In the end doing it this way took more effort than I anticipated anyways...

I'll have a look at this later. It is a massive pain to need to be maintaining multiple implementations for distributed IO, so we should see if there are reasons to keep it separate, and if we can combine any of it together.

I agree that it makes no sense the way the code is organised at the moment, and it would be much better if everything used a common approach. It might be painful to try and move things over seamlessly and support all the old formats, but just making containers for the current acquisition formats would be an improvement.

tristpinsm commented 2 years ago

ok, I think I've addressed your comments @jrs65 (thanks!). It feels like making this work is a hack, so I'm not sure what the right path forward is. I guess getting all of the CorrData code integrated with draco will not be trivial, but probably moving the other containers in here to that model would be straightforward. And this PR only addresses the latter...

tristpinsm commented 1 year ago

It's not clear this is the way forward to enable distributed reading of other acquisition files. There is a similar problem with the HFB files, so we can shelf this for now and see how the HFB approach pans out and consider generalising it to all non-visibility acquisition files.