-
Notifications
You must be signed in to change notification settings - Fork 22
Sidelobe cutter #1456
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
Sidelobe cutter #1456
Conversation
mhasself
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very useful, thanks. I know this is ported from another codebase but I think it still needs to be reformatted to match style and standards of sotodlib.
pyproject.toml
Outdated
| "psycopg2-binary", | ||
| "lmfit", | ||
| "flacarray>=0.3.4", | ||
| "numpy-quaternion", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "numpy-quaternion", | |
| "numpy-quaternion", # Required quietly by pixell.coordsys |
sotodlib/coords/sidelobes.py
Outdated
| from sotodlib import coords, mapmaking | ||
| import so3g | ||
|
|
||
| def sidelobe_cut(obs, args, sidelobe_cutters, object_list=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer active function names, like "get_". Since this is already in coords.sidelobes, consider just calling it "get_cuts(...)".
sotodlib/coords/sidelobes.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| obs : sotodlib.core.AxisManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In sotodlib we use "aman" or "tod" as the principle AxisManager to operate on.
sotodlib/coords/sidelobes.py
Outdated
| An observation axis manager. | ||
| args : dict | ||
| An argument dictionary, which must contains the path to the | ||
| masks in mask_sun and mask_moon. This is only used for these |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sun_mask and moon_mask
sotodlib/coords/sidelobes.py
Outdated
| An argument dictionary, which must contains the path to the | ||
| masks in mask_sun and mask_moon. This is only used for these | ||
| paths. | ||
| sidelobe_cutters : dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nicer if this argument were optional -- i.e. default value None, converted to {} on init. That doesn't mean you have to return the dict -- folks using this in a loop would treat it just as before by pre-instantiating the cache.
sotodlib/coords/sidelobes.py
Outdated
| first run this should be empty and will be filled. Subsequent | ||
| runs will use these precalculated objects. | ||
| object_list : list, optional | ||
| A list of the objects we want to mask. If None, sun and moon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A list of strings (lower case) indicating the objects to mask.
sotodlib/coords/sidelobes.py
Outdated
| cutss.append(cuts) | ||
| return cutss | ||
|
|
||
| def Simplecut(ndets, nsamps): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def Simplecut(ndets, nsamps): | |
| def _simple_cut(ndets, nsamps): |
And then change the two places it's called, too.
sotodlib/coords/sidelobes.py
Outdated
| def forward(tod, map, pmat): | ||
| pmat.from_map(dest=tod, signal_map=map, comps="T",) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is called exactly once, so does it need to be a function? If you keep it, make it private (_forward).
sotodlib/coords/sidelobes.py
Outdated
| except RuntimeError: | ||
| # This happens when the interpolated pointing ends up outside the -npix:+2npix range | ||
| # that gpu_mm allows. This can happen when a detector moves too close to the north pole |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The justification for this try / except is framed as a gpu_mm issue. It's not obvious that a RuntimeError raised by pmat would be due to the same cause, and thus require the same solution.
So please remove this try/except or characterize it specifically in terms of sotodlib pointing behaviors.
|
tested on a mapmaker run, good to go |
mhasself
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good!
This implements the sidelobe cutter from @amaurea sogma code. The mask is a simple enmap so it should be low-res and full sky to not be super slow. The coords module seems like the natural place, but I'm not completely sure, so open to suggestions.