Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions python/activator/middleware_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,10 @@ def prep_butler(self, visit: Visit) -> None:
wcs = self._predict_wcs(detector, visit)
center, radius = self._detector_bounding_circle(detector, wcs)

# central repo may have been modified by other MWI instances.
# TODO: get a proper synchronization API for Butler
self.central_butler.registry.refresh()

with tempfile.NamedTemporaryFile(mode="w+b", suffix=".yaml") as export_file:
with self.central_butler.export(filename=export_file.name, format="yaml") as export:
self._export_refcats(export, center, radius)
Expand Down Expand Up @@ -764,6 +768,10 @@ def _export_subset(self, visit: Visit, exposure_ids: set[int],
if not datasets:
raise ValueError(f"No datasets match visit={visit} and exposures={exposure_ids}.")

# central repo may have been modified by other MWI instances.
# TODO: get a proper synchronization API for Butler
self.central_butler.registry.refresh()

with tempfile.NamedTemporaryFile(mode="w+b", suffix=".yaml") as export_file:
# MUST NOT export governor dimensions, as this causes deadlocks in
# central registry. Can omit most other dimensions (all dimensions,
Expand Down
22 changes: 17 additions & 5 deletions python/tester/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@
_log = logging.getLogger("lsst." + __name__)
_log.setLevel(logging.DEBUG)

max_exposure = {
"HSC": 21474800,
}
"""A mapping of instrument to exposure_max (`dict` [`str`, `int`]).

The values are copied here so we can access them without a Butler. All
exposure IDs are in Middleware (integer) format, not native format.
"""


def get_last_group(bucket, instrument, date):
"""Identify the largest group number or a new group number.
Expand Down Expand Up @@ -124,15 +133,18 @@ def make_hsc_id(group_num, snap):

Notes
-----
The current implementation allows up to 1000 group numbers per day.
It can overflow with ~60 calls to upload.py on the same day or
upload_hsc_rc2.py with a large N_GROUPS.
The current implementation gives illegal exposure IDs after September 2024.
If this generator is still needed after that, it will need to be tweaked.
"""
# This is a bit too dependent on how group_num is generated, but I want the
# group number to be discernible even after compressing to 8 digits.
night_id = (group_num // 100_000) % 2020_00_00 # Always 5 digits
year = (group_num // 100_00_00000) - 2023 # Always 1 digit, 0-1
night_id = (group_num % 100_00_00000) // 100000 # Always 4 digits up to 1231
run_id = group_num % 100_000 # Up to 5 digits, but usually 2-3
exposure_id = (night_id * 1000) + (run_id % 1000) # Always 8 digits
exposure_id = (year*1200 + night_id) * 10000 + (run_id % 10000) # Always 8 digits
if exposure_id > max_exposure["HSC"]:
raise RuntimeError(f"{group_num} translated to expId {exposure_id}, "
f"max allowed is { max_exposure['HSC']}.")
return f"HSCE{exposure_id:08d}", exposure_id


Expand Down
2 changes: 1 addition & 1 deletion tests/test_middleware_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def test_get_butler(self):
]:
# TODO: better way to test repo location?
self.assertTrue(
butler.getURI("camera", instrument=instname, run="foo", predict=True).ospath
butler.getURI("skyMap", skymap="deepCoadd_skyMap", run="foo", predict=True).ospath
.startswith(self.central_repo))
self.assertEqual(list(butler.collections), [f"{instname}/defaults"])
self.assertTrue(butler.isWriteable())
Expand Down
37 changes: 36 additions & 1 deletion tests/test_tester_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,19 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <https://www.gnu.org/licenses/>.

import tempfile
import unittest

import boto3
import botocore
from moto import mock_s3

import lsst.daf.butler.tests as butler_tests
from lsst.obs.base import ExposureIdInfo
from lsst.obs.subaru import HyperSuprimeCam

from activator.raw import get_raw_path
from tester.utils import get_last_group
from tester.utils import get_last_group, make_exposure_id


class TesterUtilsTest(unittest.TestCase):
Expand Down Expand Up @@ -76,3 +81,33 @@ def test_get_last_group(self):
# Test the case of no match
last_group = get_last_group(bucket, "TestCam", "20110101")
self.assertEqual(last_group, int(20110101) * 100_000)

def test_exposure_id_hsc(self):
group = "2023011100026"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just a number, avoiding the later int()?

Copy link
Member Author

Choose a reason for hiding this comment

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

We get a lot of bugs related to integers being assigned as groups and then passed around like that, so I've insisted on other reviews that we always be strict with the types there. I could hardly make an exception for myself, especially since part of the problem is that we tend to think of group IDs as integers when formally they're not.

# Need a Butler registry to test ExposureIdInfo
with tempfile.TemporaryDirectory() as repo:
butler = butler_tests.makeTestRepo(repo)
HyperSuprimeCam().register(butler.registry)
instruments = list(butler.registry.queryDimensionRecords(
"instrument", dataId={"instrument": "HSC"}))
self.assertEqual(len(instruments), 1)
exp_max = instruments[0].exposure_max

_, str_exp_id, exp_id = make_exposure_id("HSC", int(group), 0)
butler_tests.addDataIdValue(butler, "visit", exp_id)
data_id = butler.registry.expandDataId({"instrument": "HSC", "visit": exp_id, "detector": 111})

self.assertEqual(str_exp_id, "HSCE%08d" % exp_id)
# Above assertion passes if exp_id has 9+ digits, but such IDs aren't valid.
self.assertEqual(len(str_exp_id[4:]), 8)
self.assertLessEqual(exp_id, exp_max)
# test that ExposureIdInfo.fromDataID does not raise
ExposureIdInfo.fromDataId(data_id, "visit_detector")

def test_exposure_id_hsc_limits(self):
# Confirm that the exposure ID generator works as long as advertised:
# until the end of September 2024.
_, _, exp_id = make_exposure_id("HSC", 2024093009999, 0)
self.assertEqual(exp_id, 21309999)
with self.assertRaises(RuntimeError):
make_exposure_id("HSC", 2024100100000, 0)
1 change: 1 addition & 0 deletions ups/prompt_prototype.table
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ setupRequired(pipe_base)

# used by tests
setupRequired(obs_decam)
setupRequired(obs_subaru)

envPrepend(PYTHONPATH, ${PRODUCT_DIR}/python)
envPrepend(PATH, ${PRODUCT_DIR}/bin)