You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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: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 aUVBeam
object to input (as is true in the case of thehera_sim
wrapper). Relevant info here is that theUVBeam
object can have abeam_type
of eitherpower
orefield
. The first corresponds to real-valued beam-squared (i.e.XX
orYY
) while the latter corresponds to the actual complex beam (i.e.X
and/orY
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 specifyXX
asee
ornn
. Otherwise, we don't know whatee
actually is (and it doesn't matter -- theX
andY
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:
polarization=True
andpolarization=False
, but see my later questions. If we are assuming this, we should do an assert on it.polarized=True
branch looks fine (as long as it gets used consistently through the simulation), but thepolarized=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 (possiblyphi
) and the second element of the components vector (which could be eitherX
orY
depending on how they're ordered in theUVBeam
object). Now, this is a complex value (which may or may not be whatvis_cpu
expects), but its square is not theXX
orYY
beam. 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 thephi
vector component of theX
feed. The true square of theX
feed should bephi^2 + theta^2
, right?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.
The text was updated successfully, but these errors were encountered: