Mapcat integration for make_atomic_filterbin_map and make_coadd_atomic_map by davidvng · Pull Request #1534 · simonsobs/sotodlib

davidvng · 2026-01-22T21:32:57Z

Modifies existing database querying and writing from original sqlite structure to using mapcat for both make_atomic_filterbin_map and make_coadd_atomic_map

Related changes will be needed in the site-pipeline-configs, especially to set the following environment variables used by mapcat:

MAPCAT_DATABASE_NAME
MAPCAT_DATABASE_TYPE
MAPCAT_ATOMIC_PARENT
MAPCAT_ATOMIC_COADD_PARENT

I've tested the coadd script and confirmed everything works as intended.

However, for the atomic mapmaking, I have not tested due to permissions issues. I'd appreciate @chervias or @JBorrow to double check the changes in make_atomic_filterbin_map.py make sense.

JBorrow · 2026-01-22T22:07:48Z

Not sure why the tests are failing due to the missing dependency.

iparask · 2026-01-23T14:09:23Z

Hello @davidvng, take a look here to see how mapcat (#1485) is integrated with the depth1 mapmaker. There we pass the config as an input. In addition you do not really need select as the session provides a query method which is as easy to invoke as select.

JBorrow · 2026-01-23T14:12:34Z

@iparask using session.query() is deprecated. Forming a select statement is preferred behaviour and is the standard in SQLAlchemy 2.0+. See: https://docs.sqlalchemy.org/en/21/changelog/migration_20.html#migration-orm-usage

iparask · 2026-01-23T14:18:34Z

+        with settings.session() as session:
+            data = AtomicMapCoaddTable(
+                coadd_name=map_name,
+                prefix_path=prefix_path,
+                platform=platform,
+                interval=interval,
+                start_time=start_time,
+                stop_time=stop_time,
+                freq_channel=band,
+                geom_file_path=self.geom_file_path,
+                split_label=split_label,
+                atomic_maps=self.maps if self.coadd_atomic else [],
+                parent_coadds=self.maps if not self.coadd_atomic else []
+            )
+
+            session.add(data)
+            session.commit()


I would place this in a function similar to the depth 1 map in the PR I referenced

iparask · 2026-01-23T14:19:19Z

+        with settings.session() as session:
+            if '+' in band:
+                statement = select(AtomicMapTable).where((AtomicMapTable.telescope == platform) & (AtomicMapTable.split_label == split_label) & (AtomicMapTable.ctime >= start_time) & (AtomicMapTable.ctime <= stop_time))
+            else:
+                statement = select(AtomicMapTable).where((AtomicMapTable.telescope == platform) & (AtomicMapTable.freq_channel == band) & (AtomicMapTable.split_label == split_label) & (AtomicMapTable.ctime >= start_time) & (AtomicMapTable.ctime <= stop_time))
+            maps = session.execute(statement).scalars().all()
+            map_ids = [m.atomic_map_id for m in maps]


The same as above

iparask · 2026-01-23T14:20:47Z

+        with settings.session() as session:
+            statement = select(AtomicMapCoaddTable).where((AtomicMapCoaddTable.interval == interval) & (AtomicMapCoaddTable.platform == platform) & (AtomicMapCoaddTable.freq_channel == band) & (AtomicMapCoaddTable.split_label == split_label) & (AtomicMapCoaddTable.start_time == start_time) & (AtomicMapCoaddTable.stop_time == stop_time)).order_by(AtomicMapCoaddTable.coadd_id.desc())
+            coadd = session.execute(statement).scalars().first()
+            if coadd is not None:
+                atomic_parent_ids = [a.atomic_map_id for a in coadd.atomic_maps]
+                coadd_parent_ids = [c.coadd_id for c in coadd.parent_coadds]
+
+                if (map_ids == atomic_parent_ids) or (map_ids == coadd_parent_ids):
+                    return False
+


This can also be a function under mapcat.py

davidvng · 2026-02-02T18:34:27Z

Not sure why the tests are failing due to the missing dependency.

@JBorrow The tests are still failing and I'm assuming it has to do with mapcat being in the optional dependencies? Thoughts on moving it to the core list of dependencies?

iparask · 2026-02-02T18:58:53Z

Not sure why the tests are failing due to the missing dependency.

@JBorrow The tests are still failing and I'm assuming it has to do with mapcat being in the optional dependencies? Thoughts on moving it to the core list of dependencies?

I am not sure that is the solution. #759 passes with mapcat as an optional dependency. It might be some other reason. There is similar discussion on slack.

davidvng · 2026-02-04T19:26:25Z

@JBorrow @iparask Looks like the python-3.13 tests now pass, but the python-3.10 tests will always fail since mapcat doesn't support 3.10.

JBorrow · 2026-02-04T21:35:39Z

0.1.1 includes 'support' for 3.10.

iparask · 2026-02-13T19:41:51Z

+        with Settings(**mapcat_settings).session() as session:
+            data = AtomicMapCoaddTable(
+                coadd_name=map_name,
+                prefix_path=prefix_path,
+                platform=platform,
+                interval=interval,
+                start_time=start_time,
+                stop_time=stop_time,
+                freq_channel=band,
+                geom_file_path=self.geom_file_path,
+                split_label=split_label,
+                atomic_maps=self.maps if self.coadd_atomic else [],
+                parent_coadds=self.maps if not self.coadd_atomic else []
+            )
+
+            session.add(data)
+            session.commit()


This should be outside of the mapmaking module. Think what will happen if we decide to change mapcat with something else. We will have to change the code here as well.

Please move any database interactions in the site_pipeline scripts, and create a file for these interactions under the utils of site_pipeline to keep them.

…omic-mapcat

iparask · 2026-03-04T19:24:36Z

This line is left there from the direct integration with sqlite.

iparask · 2026-03-04T23:50:55Z

It seems that this is not used anymore. I think you can remove it.

* fix: mapcat integration with atomic maps * fix: no need to return everything

iparask · 2026-04-22T15:45:15Z

@JBorrow, @chervias can you also take a look here?

JBorrow · 2026-04-23T19:50:34Z

+        mapcat_database_name: str = os.environ.get('MAPCAT_DATABASE_NAME', ''),
+        mapcat_database_type: str = os.environ.get('MAPCAT_DATABASE_TYPE', ''),
+        mapcat_atomic_parent: str = os.environ.get('MAPCAT_ATOMIC_PARENT', ''),


You already have mapcat settings here so just use that, it reads the environment for you.

JBorrow · 2026-04-23T19:51:20Z

Other than the duplicated reading of environment variables (you should use Settings from mapcat as the variables may be specified in another manner) this looks good.

JBorrow · 2026-05-07T13:26:22Z

-    mapcat_settings = {"database_type": args.mapcat_database_type,
-                       "database_name": args.mapcat_database_name,
-                       "atomic_parent": args.mapcat_atomic_parent,}
+    mapcat_settings = {"database_type": Settings().database_type,


I don't understand this at all, why are we reading data out of the object into a dictionary with the same keys?

The dictionary is later passed to here:

sotodlib/sotodlib/site_pipeline/utils/mapcat.py

Line 198 in 85240a7

mapcat_settings: Dict[str, str],

The other functions, the depth-1 ones for example, were written in this way too. I could change the atomic functions here to directly use Settings() but that would make them different from the depth-1 ones.

mapcat_settings = Settings().model_dump(include=[ "database_type", "database_name", "atomic_parent" ])

Is a reasonable compromise :).

JBorrow · 2026-05-12T18:49:24Z

+        if mapcat_database_name is None:
+            mapcat_database_name = Settings().database_name
+        if mapcat_database_type is None:
+            mapcat_database_type = Settings().database_type
+        if mapcat_atomic_parent is None:
+            mapcat_atomic_parent = Settings().atomic_parent
+        if mapcat_atomic_coadd_parent is None:
+            mapcat_atomic_coadd_parent = Settings().atomic_coadd_parent
+        self.mapcat_settings = Settings(database_name = mapcat_database_name,
+                                        database_type = mapcat_database_type,
+                                        atomic_parent = mapcat_atomic_parent,
+                                        atomic_coadd_parent = mapcat_atomic_coadd_parent)
+


Suggested change

if mapcat_database_name is None:

mapcat_database_name = Settings().database_name

if mapcat_database_type is None:

mapcat_database_type = Settings().database_type

if mapcat_atomic_parent is None:

mapcat_atomic_parent = Settings().atomic_parent

if mapcat_atomic_coadd_parent is None:

mapcat_atomic_coadd_parent = Settings().atomic_coadd_parent

self.mapcat_settings = Settings(database_name = mapcat_database_name,

database_type = mapcat_database_type,

atomic_parent = mapcat_atomic_parent,

atomic_coadd_parent = mapcat_atomic_coadd_parent)

# Allow for CLI over-rides of environment variables

self.mapcat_settings = Settings(

**{

k: v for k, v in {

"database_name": mapcat_database_name,

"database_type": mapcat_database_type,

"atomic_parent": mapcat_atomic_parent,

"atomic_coadd_parent": mapcat_atomic_coadd_parent,

}.items() if v is not None

}

)

JBorrow · 2026-05-12T18:49:58Z

+
+        if mapcat_database_name is None:
+            mapcat_database_name = Settings().database_name
+        if mapcat_database_type is None:
+            mapcat_database_type = Settings().database_type
+        if mapcat_atomic_parent is None:
+            mapcat_atomic_parent = Settings().atomic_parent
+        self.mapcat_settings = Settings(database_name = mapcat_database_name,
+                                        database_type = mapcat_database_type,
+                                        atomic_parent = mapcat_atomic_parent)


Suggested change

if mapcat_database_name is None:

mapcat_database_name = Settings().database_name

if mapcat_database_type is None:

mapcat_database_type = Settings().database_type

if mapcat_atomic_parent is None:

mapcat_atomic_parent = Settings().atomic_parent

self.mapcat_settings = Settings(database_name = mapcat_database_name,

database_type = mapcat_database_type,

atomic_parent = mapcat_atomic_parent)

# Allow for CLI over-rides of environment variables

self.mapcat_settings = Settings(

**{

k: v for k, v in {

"database_name": mapcat_database_name,

"database_type": mapcat_database_type,

"atomic_parent": mapcat_atomic_parent,

}.items() if v is not None

}

)

iparask · 2026-05-13T14:17:20Z

@davidvng can you merge with master and the tests will pass.

davidvng added 4 commits January 22, 2026 20:02

atomic coadd mapcat integration

ca4eebc

atomic filterbin mapcat integration

7afe8f1

import session cleanup

8d1de9f

fix query typo

f447c02

davidvng requested review from JBorrow and chervias January 22, 2026 21:32

add mapcat module

178f7d5

JBorrow reviewed Jan 22, 2026

View reviewed changes

Comment thread sotodlib/mapmaking/coadd_mapmaker.py Outdated

iparask reviewed Jan 23, 2026

View reviewed changes

cleanup statements

a69c373

davidvng added 2 commits February 2, 2026 19:09

Merge branch 'master' into atomic-mapcat

6b4b56b

Merge branch 'master' into atomic-mapcat

7a9f308

iparask mentioned this pull request Feb 4, 2026

Include python3.10 simonsobs/mapcat#17

Merged

davidvng added 5 commits February 9, 2026 20:09

Merge branch 'master' into atomic-mapcat

694d99b

using mapcat settings from config file; various cleanup

35d05c8

remove print statements

9023240

use mapcat settings from configs for atomic mapmaking

dbe1a8b

Merge branch 'master' into atomic-mapcat

96c252c

davidvng requested review from JBorrow and iparask February 9, 2026 22:49

iparask requested changes Feb 13, 2026

View reviewed changes

davidvng added 2 commits March 3, 2026 20:57

Merge branch 'master' into atomic-mapcat

bbaf515

Merge branch 'atomic-mapcat' of github.com:simonsobs/sotodlib into at…

dc0d756

…omic-mapcat

iparask reviewed Mar 4, 2026

View reviewed changes

iparask and others added 3 commits March 9, 2026 14:50

Integrating mapcat with atomic maps (#1569)

5576c3e

* fix: mapcat integration with atomic maps * fix: no need to return everything

fix params

2a8edc1

remove unused sqlite3

dab1b84

iparask self-requested a review April 22, 2026 15:44

JBorrow reviewed Apr 23, 2026

View reviewed changes

davidvng added 2 commits April 30, 2026 19:19

Merge branch 'master' into atomic-mapcat

85dbc5e

use mapcat settings in atomic

85240a7

JBorrow reviewed May 7, 2026

View reviewed changes

davidvng added 3 commits May 12, 2026 18:32

adjust mapcat settings usage

d4582a4

add update_delay similar to atomic mapmaking script

9835dff

Merge branch 'master' into atomic-mapcat

7233dee

JBorrow reviewed May 12, 2026

View reviewed changes

cleaner mapcat settings

ef4463a

Merge branch 'master' into atomic-mapcat

99bff06

Conversation

davidvng commented Jan 22, 2026

Uh oh!

JBorrow commented Jan 22, 2026

Uh oh!

Uh oh!

iparask commented Jan 23, 2026

Uh oh!

JBorrow commented Jan 23, 2026

Uh oh!

iparask Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidvng commented Feb 2, 2026

Uh oh!

iparask commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidvng commented Feb 4, 2026

Uh oh!

JBorrow commented Feb 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iparask commented Apr 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JBorrow commented Apr 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidvng May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JBorrow May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JBorrow May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iparask commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

iparask Jan 23, 2026 •

edited

Loading

iparask commented Feb 2, 2026 •

edited

Loading

davidvng May 8, 2026 •

edited

Loading

JBorrow May 12, 2026 •

edited

Loading

JBorrow May 12, 2026 •

edited

Loading