Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(galt_auto_cont): Add container and task for saving raw Galt autos. #109

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions ch_pipeline/core/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,39 @@ def source(self):
return self.index_map["source"]


class GaltAutocorrelation(ContainerBase):
_axes = ("freq", "pol", "time")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support common axes like freq and time, there are base classes in draco that you should inherit from. draco.core.containers.FreqContainer and draco.core.containers.TODContainer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_axes = ("freq", "pol", "time")
_axes = ("pol",)

If you inherit from the base classes, then you no longer need to specify the axes covered by those classes. A new instance will have all the unique axes contained in all subclasses.


_dataset_spec = {
"auto": {
"axes": ["freq", "pol", "time"],
"dtype": np.complex64,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this container is explicitly for autos, this probably doesn't need to be complex

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but in principle we also want the cross-correlation of the two 26m polarisations in addition to 26m XX and YY, and that has a phase, so I set the container to be complex.

"initialise": True,
"distributed": True,
"distributed_axis": "freq",
},
"weight": {
"axes": ["freq", "pol", "time"],
"dtype": np.float64,
"initialise": True,
"distributed": True,
"distributed_axis": "freq",
},
}

@property
def auto(self):
return self.datasets["auto"]

@property
def weight(self):
return self.datasets["weight"]

@property
def fpga_count(self):
return self.index_map["time"]["fpga_count"]


def make_empty_corrdata(
freq=None,
input=None,
Expand Down
50 changes: 49 additions & 1 deletion ch_pipeline/core/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@

from draco.core import task, io


class LoadCorrDataFiles(task.SingleTask):
"""Load data from files passed into the setup routine.

Expand Down Expand Up @@ -473,3 +472,52 @@ def next(self, files):
)

return sorted(list(new_files))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these new lines can be be removed so that this PR does not modify the io module.

class SaveGaltAutoCorrelation(task.SingleTask):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is a bit nit-picky, but I would suggest renaming this to something like ExtractGaltAutocorrelationand moving it out of io (since it only performs io if you choose to save the output, same as any task). I'm not sure where would be the best place for it... Maybe analysis.calibration since I assume this is meant for noise source analysis. analysis.beam could also work.

"""Extract the autocorrelations of the Galt telescope from a holography acquisition."""

# YY, YX, XX
_galt_prods = [2450, 2746, 3568]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would probably be preferable to determine these from the index_map rather than hard code them. I don't think these indices would have changed, but in principle they are allowed to.


def process(self, data):
"""Extract the Galt autocorrelations and write them to disk.
Parameters
----------
data: TimeStream
A TimeStream container holding a raw holography acquisition.
Returns
-------
autocorrelation: containers.GaltAutocorrelation
A GaltAutocorrelation container holding the extracted Galt autos
as a function of frequency, polarization product, and time.
"""
from .containers import GaltAutocorrelation

# Redistribute over freq
data.redistribute("freq")

# Dereference beam and weight datasets
beam = data.vis[:].view(np.ndarray)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the preferred way to this now is to use local_array

Suggested change
beam = data.vis[:].view(np.ndarray)
beam = data.vis[:].local_array

weight = data.weight[:].view(np.ndarray)

# Load only the data corresponding to the Galt inputs
galt_auto = beam[:, self._galt_prods, :]
galt_weight = weight[:, self._galt_prods, :]

# Initialize the auto container
autocorrelation = GaltAutocorrelation(
pol=np.array([b"YY", b"YX", b"XX"]),
attrs_from=data,
freq=data.freq,
time=data.index_map["time"],
comm=data.comm,
distributed=data.distributed,
)

# Redistribute output container over frequency
autocorrelation.redistribute("freq")

autocorrelation.auto[:] = galt_auto
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure but I think this might trigger a warning with the new mpiarray... The alternative would be to use local_array again.

Suggested change
autocorrelation.auto[:] = galt_auto
autocorrelation.auto[:].local_array[:] = galt_auto

But if it works without generating warnings with the latest caput I guess it's fine.

autocorrelation.weight[:] = galt_weight

return autocorrelation