I think the default case of polarization=False is doing something wrong, or at the very least should be clarified. For reference, here's the lines I'm most concerned with:
These are in the uvbeam_to_lm() function, which is a very useful function if the user has a UVBeam object to input (as is true in the case of the hera_sim wrapper). Relevant info here is that the UVBeam object can have a beam_type of eitherpower or efield. The first corresponds to real-valued beam-squared (i.e. XX or YY) while the latter corresponds to the actual complex beam (i.e. X and/or Y and the two vector components associated with each).
Note also that we may or may not have a x_orientation set which (if set) would enable to specify XX as ee or nn. Otherwise, we don't know what ee actually is (and it doesn't matter -- the X and Y are just labels, and don't actually affect any of the simulation calculations).
So, with that info there are a few questions/issues to be addressed:
There are no checks/assertions for whether the beam is type 'power' or 'efield'. It seems like the assumption in the code is that it's 'efield' both for polarization=True and polarization=False, but see my later questions. If we are assuming this, we should do an assert on it.
The polarized=True branch looks fine (as long as it gets used consistently through the simulation), but the polarized=False branch seems wrong to me. Under the assumption that the beam is efield, it looks like what we're taking is the first element of the axis vector (possibly phi) and the second element of the components vector (which could be either X or Y depending on how they're ordered in the UVBeam object). Now, this is a complex value (which may or may not be what vis_cpu expects), but its square is not the XX or YY beam. Let's say we picked out phi, X (as I said, whether this is true depends on the ordering of the axes, but let's say it is true for now): then the square of this (which is done inside vis_cpu() is just the square of the phi vector component of the X feed. The true square of the X feed should be phi^2 + theta^2, right?
It seems that, according to the comments in the above code, we are assuming that we are getting the east feed. However, this just may not be true -- it'll depend on the ordering of the feeds in the object (if it mattered, we could just query the object for which is east and grab that one). However, I think it doesn't matter -- I don't think anything in the simulation depends on the feed being east or north or anything in between for that matter. As long as the beam interpolation onto az/za is appropriate, that's all we care about. So I suggest we replace this comment.
I think it would be most fruitful if we can figure out what to do live via zoom, and maybe hack out the solution at the same time. If @philbull and @bhazelton could join that would be awesome!
This issue is related to #27 and #15.
I think the default case of
polarization=Falseis doing something wrong, or at the very least should be clarified. For reference, here's the lines I'm most concerned with:https://github.com/HERA-Team/vis_cpu/blob/687cb7c92e30b5aee4b094304e63753c8966d63f/src/vis_cpu/conversions.py#L300-L304
These are in the
uvbeam_to_lm()function, which is a very useful function if the user has aUVBeamobject to input (as is true in the case of thehera_simwrapper). Relevant info here is that theUVBeamobject can have abeam_typeof eitherpowerorefield. The first corresponds to real-valued beam-squared (i.e.XXorYY) while the latter corresponds to the actual complex beam (i.e.Xand/orYand the two vector components associated with each).Note also that we may or may not have a
x_orientationset which (if set) would enable to specifyXXaseeornn. Otherwise, we don't know whateeactually is (and it doesn't matter -- theXandYare just labels, and don't actually affect any of the simulation calculations).So, with that info there are a few questions/issues to be addressed:
polarization=Trueandpolarization=False, but see my later questions. If we are assuming this, we should do an assert on it.polarized=Truebranch looks fine (as long as it gets used consistently through the simulation), but thepolarized=Falsebranch seems wrong to me. Under the assumption that the beam is efield, it looks like what we're taking is the first element of the axis vector (possiblyphi) and the second element of the components vector (which could be eitherXorYdepending on how they're ordered in theUVBeamobject). Now, this is a complex value (which may or may not be whatvis_cpuexpects), but its square is not theXXorYYbeam. Let's say we picked outphi, X(as I said, whether this is true depends on the ordering of the axes, but let's say it is true for now): then the square of this (which is done insidevis_cpu()is just the square of thephivector component of theXfeed. The true square of theXfeed should bephi^2 + theta^2, right?eastfeed. However, this just may not be true -- it'll depend on the ordering of the feeds in the object (if it mattered, we could just query the object for which is east and grab that one). However, I think it doesn't matter -- I don't think anything in the simulation depends on the feed being east or north or anything in between for that matter. As long as the beam interpolation onto az/za is appropriate, that's all we care about. So I suggest we replace this comment.I think it would be most fruitful if we can figure out what to do live via zoom, and maybe hack out the solution at the same time. If @philbull and @bhazelton could join that would be awesome! This issue is related to #27 and #15.