Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(beam): use GridBeam in telescope coordinates #238

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ssiegelx
Copy link
Contributor

Allows the user to generate a HybridVisStream from a GridBeam in telescope coordinates. This avoids problems at the pole when using GridBeams in celestial coordinates that do not span 360 degrees in hour angle.

Allows the user to generate a HybridVisStream from a GridBeam
in telescope coordinates.  This avoids problems at the pole
when using GridBeams in celestial coordinates that do not span
360 degrees in hour angle.
@ssiegelx ssiegelx requested a review from tristpinsm April 11, 2023 19:55
@ssiegelx
Copy link
Contributor Author

I have tested the code in a notebook, but have not yet attempted to submit jobs that use this task or examine the results in detail.

The updated task should give the same results if provided a GridBeam in celestial coordinates. However, it was restructured a fair amount, so it's possible there will be some unintended changes. We should confirm that we still get the same result for this case.

Copy link
Contributor

@tristpinsm tristpinsm left a comment

Choose a reason for hiding this comment

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

Looks good! I will remake my plots with this code and post the result here.

@tristpinsm
Copy link
Contributor

tristpinsm commented Apr 11, 2023

Here are plots generated using the telescope beam input to CreateBeamStream. The ringing near the NCP is a lot more localised in declination, but still present. Also, whereas before the NCP itself appeared to be masked, that no longer seems to be the case.
mmode_beam_477_Y
mmode_beam_477_X

I generated these in a notebook, but I also submitted a job to make maps using the celestial beam as before. Once this is done I can compare to the maps generated from the old code to see if there is a difference.

@ssiegelx
Copy link
Contributor Author

Thanks @tristpinsm. I think the remaining ringing in declination is due to the |hour angle| < 90 degree limit that is placed to ignore the antipodal transit. There is roughly +/- 2.5 degrees around the NCP where the beam response is significant at 90 degrees.

I'm not sure there are any great options as that assumptions that are made in the beam generation begin to break down as you approach the NCP. Perhaps we should just set the weight dataset for the beam stream to zero for these declinations?

@tristpinsm
Copy link
Contributor

I'm not sure there are any great options as that assumptions that are made in the beam generation begin to break down as you approach the NCP. Perhaps we should just set the weight dataset for the beam stream to zero for these declinations?

Yeah that seems reasonable, there is not much sky area there anyway...


# Create output container
out = containers.HybridVisStream(
ra=nra,
ra=ra,
Copy link
Contributor

Choose a reason for hiding this comment

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

In the celestial beam case, ra is defined based on the phi axis of the beam container, which doesn't span the full RA range and causes a crash later on. I will fix this and set the test job running again.

Comment on lines 84 to 86
ra = (ha + 360.0) % 360.0
nra = int(round(360.0 / np.abs(ha[1] - ha[0])))
delta_ra = 360.0 / nra
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
ra = (ha + 360.0) % 360.0
nra = int(round(360.0 / np.abs(ha[1] - ha[0])))
delta_ra = 360.0 / nra
ra_beam = (ha + 360.0) % 360.0
nra = int(round(360.0 / np.abs(ha[1] - ha[0])))
delta_ra = 360.0 / nra
ra = np.arange(nra) * delta_ra

With this change (and propagating ra_beam in the next few lines) the code ran to completion and I confirmed that the result is the same as with the previous version by comparing the ringmap and weights for about 20 unflagged randomly selected frequencies in the lower band.

@ssiegelx
Copy link
Contributor Author

@Arnab-half-blood-prince This PR enables CreateBeamStream to handle GridBeam containers in telescope coordinates. Unfortunately it never got merged and I don't remember the status of testing. Do you want to try running it?

@tristpinsm
Copy link
Contributor

tristpinsm commented Jun 14, 2024

I can push the changes I proposed here, and I think with those it was working well. I see that is already done. Nice.

Older versions of scipy do not support use of RectBivariateSpline
interpolation on complex numbers.  Perform real and imaginary
interpolation separately.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants