Skip to content

Adding unfied planet mapmaker using for MF-SATs - #1706

Draft
yoshinori-0778 wants to merge 2 commits into
masterfrom
add_planet_mapmaker
Draft

Adding unfied planet mapmaker using for MF-SATs#1706
yoshinori-0778 wants to merge 2 commits into
masterfrom
add_planet_mapmaker

Conversation

@yoshinori-0778

Copy link
Copy Markdown
Contributor

Adding general functions in coords.planets.py and functions related to planet mapmaking in mapmaking.planet_mapmaker.py.

Now is a very very draft, but I confirmed that a planet map was made. Will arrange these more appropriately.

@yoshinori-0778
yoshinori-0778 requested a review from mhasself August 4, 2026 06:14
"""
# This is based on TOD fits summarized here: https://docs.google.com/presentation/d/1DPBeMX9ZPHwQyi9-s5orfRZIYcRLXaejFWOtlsflgUU/edit?usp=sharing
# amplitude below is in arcmin, phase in radian
amp_satp1 = {'f090': {'ws0': 0.3945,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these sorts of hard-codings I would move to a version-controlled configuration file.

@mhasself mhasself left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for all this work! As requested, here is some general guidance (but not an exhaustive review yet):

  • Please include an example config file as part of documentation.
  • Format code and docs to less than 120 columns.
  • There are also some "whitespace" violations -- might be worth just running a source formatter on the new files you're contributing (but don't mass-reformat the existing files like planets.py!)
  • Consider making the deflection/wobble code a little more general, and moving into coords module somewhere.
  • You have functions for getting and archiving and processing PWV. Are those needed, if instead we can provide PWV through obsdb? I think it's good to minimize the number of different ways we pull data from APEX / Toco radiometer.
  • Use "single" instead of "each" for functions that process one of something (e.g. get_each_instrument_P). "Each" implies that it is looping, internally, over a bunch of things.
  • Remove default paths from planet_mapmaker.py funcs

Much of the new code in planets.py is basically dealing with the fact that SlowSource is not really good enough, at this point. It would be nice to take your functions calc_planet_* and instead create a replacement for SlowSource, e.g. AzElSource, that provides an az, el interpolator for any kind of source with sufficient precision for beams work. (Basically I want to get rid of SlowSource in favor of something that works better, without a huge performance penalty for simple cases.)

tags = obs['tags']
subs = 'ws'
if configs['query'].get('all_wafers', False):
obs_wafers = [f'ws{i}' for i in range(7)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps use obs['wafer_slots_list'] for this.

# add function for overwrite
for wafer in obs_wafers:
if configs['overwrite']:
irunlist = {'obs_id':obs_id, 'wafer_info': {'wafer_slot': wafer, 'wafer.bandpass': band}}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Construct this data one time (instead of duplicating 3 times).

@@ -0,0 +1,138 @@
# To make atmic planet map in detector-centered coordinate system

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use a full submodule docstring """ ... """ here, and explain in more detail what the script does. It's also a good place to put in an example of a configuration file. The docstring here is easy to have rendered in readthedocs, so it can be the "one place" this script is explained.

'det_weights': det_weights,
}

def xieta0(q):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, xieta decomposition won't work (as you say in docstring), but I think this would?:

 phi, theta, gamma = so3g.proj.quat.decompose_iso(q)
 return so3g.proj.rotation_iso(phi, theta, 0.)

@mhasself

mhasself commented Sep 2, 2026

Copy link
Copy Markdown
Member

Thanks for all this work! As requested, here is some general guidance (but not an exhaustive review yet):
...

One more thing: utility functions (such as the ones you've added in coords.planets) should have unit tests. (It's not necessary to duplicate all the calculations; rather the tests just make sure there aren't typos and that different ways of asking for the same thing will yield the same answer.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants