Skip to content

Ys/planet db - #38

Open
yoshinori-0778 wants to merge 5 commits into
mainfrom
ys/planet_db
Open

yoshinori-0778 wants to merge 5 commits into
mainfrom
ys/planet_db

Conversation

@yoshinori-0778

@yoshinori-0778 yoshinori-0778 commented Sep 7, 2026 •

Copy link
Copy Markdown
Collaborator

This PR adds database for planet maps and todfit. These are used in planet mapmaker in sotodlib (simonsobs/sotodlib#1736) for MF-SATs.

Yoshinori Sueno added 2 commits September 23, 2026 21:50
@yoshinori-0778
yoshinori-0778 marked this pull request as ready for review September 24, 2026 13:53

@JBorrow JBorrow 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.

I'd like to see more documentation on the fields and a potential consideration of creating more tables that are linked to keep individual row size smaller.

Qmap_variance: float | None
Umap_variance: float | None

proc: dict[str, Any] | list[Any] | None = None

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.

This won't serialize nicely to the database. What is proc, and what should it contain?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

preprocess name that was used to make a map. E.g.,

['hwp_angle', 'smurfgaps', 'darks', 'fp_flags', 'turnaround_flags', 'det_bias_flags', 'source_flags', 'source_flags_moon', 'trends', 'jumps_slow', 'jumps_2pi', 'glitches_pre_hwpss', 'pre_hwpss_stats', 'glitches_post_hwpss', 'post_hwpss_stats', 'ptp_flags', 'white_noise_nofit', 'lpf_sig_run1', 'noiseQ_nofit', 'noiseU_nofit', 'demodQ_glitches', 'demodU_glitches']

Comment on lines +19 to +20
ctime: AstroPydanticTime
dtime: datetime

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.

We generally like to keep all times internally as astropydantic times.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Got it, so should I remove dtime?

Comment on lines +87 to +92
obs_id: str = Field(primary_key=True)
telescope: str = Field(primary_key=True)
freq_channel: str = Field(primary_key=True)
wafer: str = Field(primary_key=True)
ctime: float = Field(nullable=False, primary_key=True)
source: str = Field(primary_key=True)

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.

This is a very large composite primary key. I would have thought that the obs_id would be unique enough, but I guess it may not be?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes, obs_id is not unique enough. I realize that ctime is actually not needed to be primary_key. I will remove that. I would like to keep other keys.

telescope: str = Field(primary_key=True)
freq_channel: str = Field(primary_key=True)
wafer: str = Field(primary_key=True)
ctime: float = Field(nullable=False, primary_key=True)

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.

You have redeclared ctime from a time to a float here.

redchiu: float | None


class PlanetMapTable(SQLModel, table=True):

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.

It would be great to have some documentation on some of these fields.

Comment on lines +62 to +70
# Main beam fit result
amplitude: float | None
peak: float | None
xo: float | None
yo: float | None
sigmax: float | None
sigmay: float | None
theta: float | None
redchit: float | None

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.

Is it possible that we could have this as a separate link table? That's especially useful if a separate process comes by and does the beam fitting at a later time, or the beam fitting might be re-done several times. See e.g. https://github.com/simonsobs/mapcat/blob/main/mapcat/database/pointing_residual.py

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I will make a separte link for main beam and leakage beam.

Comment on lines +72 to +81
mq: float | None
d0q: float | None
d1q: float | None
sigmaq: float | None
redchiq: float | None
mu: float | None
d0u: float | None
d1u: float | None
sigmau: float | None
redchiu: float | None

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.

Same question about a linked table here.

This branch has not been deployed

No deployments
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.

2 participants