New SSOPatch offset_from_sso option #810
Conversation
src/toast/schedule_sim_ground.py
Outdated
| try: | ||
| self.body = getattr(ephem, name)() | ||
| except: | ||
| raise RuntimeError("Failed to initialize {} from pyEphem".format(name)) | ||
| self.corners = None | ||
| if (self.offset_from_sso is not None) and len(self.offset_from_sso)!=2: |
There was a problem hiding this comment.
Storing the offset as a tuple means that you have to parse it every time you need to apply it.
Instead, can you try adding a try-except statement where you parse offset_from_sso into self.offset_az and self.offset_el and raise an appropriate exception when that fails?
There was a problem hiding this comment.
yes that seems better.
src/toast/schedule_sim_ground.py
Outdated
| if self.offset_from_sso: | ||
| xi_off = self.offset_from_sso[0] | ||
| eta_off = self.offset_from_sso[1] | ||
| az_newcen = xi_off + self.body.az |
There was a problem hiding this comment.
Thinking about this a little deeper now.. Does this really work? On-sky, the provided offset should always translate to the same angular distance. However, if you apply the offset in Az/El, the distance can be tiny when the telescope is pointing high in elevation. You probably need to apply the offset using pointing vectors and quaternions.
There was a problem hiding this comment.
Looking at the coordinate pdf it has the following line page 10:
"One often encounters the approximation that amounts to ξ = −∆az/ cos(el), η = ∆el."
Maybe if I implement that it is good enough?
There was a problem hiding this comment.
It is a little bit crude for large focalplanes like the SAT but we can certainly test it first.
|
@tskisner Tagging you for advice on coordiate transformations |
There was a problem hiding this comment.
Very minor requests for the code. I would like to see a simulation that verifies the offsets are working as intended. See https://github.com/simonsobs/pwg-scripts/blob/master/pwg-tds/sim-sso/run.lat.sh
| @@ -2654,7 +2652,7 @@ def parse_args(opts=None): | |||
| --patch name,weight,lon,lat,radius (center and radius) | |||
| --patch name,weight,lon_min,lat_max,lon_max,lat_min (rectangle) | |||
| --patch name,weight,lon1,lat1,lon2,lat2,...,lonN,latN (polygon, N>=3) | |||
| --patch name,SSO,weight,radius (Solar System Object) | |||
| --patch name,SSO,weight,radius,eta_off,xi_off (Solar System Object, last two optional offset for pointingnat specific place on fp) | |||
There was a problem hiding this comment.
Can you add square brackets around the optional fields: ,eta_off,xi_off => [,eta_off,xi_off] so it is immediately obvious they are optional?
There is also a missing space at pointingat.
| el_newcen = eta_off + self.body.alt | ||
| ra, dec = observer.radec_of(az_newcen, el_newcen) | ||
| if self.offset_eta !=0 or self.offset_xi !=0: | ||
| delta_az = -self.offset_xi*np.cos(self.body.alt) |
There was a problem hiding this comment.
We can use this approximation for now, but you should add a comment about it.
There was a problem hiding this comment.
Looking at the sim-sso/run.lat.sh I am a bit confused by how the boresight_offset arguments gel with our new offset of patches
There was a problem hiding this comment.
This the the opposite of what you are doing since your offset applies to the target. If, for example, the scheduler was told to only schedule el=50 scans, applying boresight offset in the old implementation would lead to the boresight actually being at a different elevation.
The way you implemented the offset, elevation constaints will be observed and we can specify separate offsets for different targets.
Like Katie asked. not sure about the xi/eta-> az/el correspondence.