-
Notifications
You must be signed in to change notification settings - Fork 10
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
DM-25890: For gen2to3 determine gen3 instrument from gen2 mapper #273
Conversation
# Derive the gen3 instrument from the gen2root | ||
# This requires we instantiate a gen2 butler solely to get its mapper | ||
with temporaryLogLevel("CameraMapper", Log.ERROR): | ||
with temporaryLogLevel("HscMapper", Log.ERROR): |
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 seems kind of specific. Is this the only mapper logger that doesn't use CameraMapper
?
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.
I was wondering about that. It's the code that is in the other bit of the conversion code that I lifted it from. It seemed like @TallJimbo needed that in one place so I'm going to need it here. It's a shame the mappers don't share a log context. obs_lsst uses "LsstCamMapper"
so I probably need to suppress that as well (since it looks like it warns in the constructor). Or we remove HscMapper and live with the irrelevant chatter.
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.
I think I should just do this for the root mapper instead and hide everything. The system will issue the warnings a bit later anyhow. Talking of which, on my laptop it takes 10 seconds to load the imsim mapper and 14 seconds for the ts3 mapper. It's a shame there's no way to short circuit the yaml camera parsing or defer it until we need it. (80% of the time is spent calling _makeCamera).
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.
I think I can make the time penalty disappear if I make this change to obs_lsst:
diff --git a/python/lsst/obs/lsst/lsstCamMapper.py b/python/lsst/obs/lsst/lsstCamMapper.py
index 9db3744..44e433b 100644
--- a/python/lsst/obs/lsst/lsstCamMapper.py
+++ b/python/lsst/obs/lsst/lsstCamMapper.py
@@ -23,6 +23,7 @@
"""The LsstCam Mapper.""" # necessary to suppress D100 flake8 warning.
import os
+from functools import lru_cache
import lsst.log
import lsst.geom
import lsst.utils as utils
@@ -166,7 +167,13 @@ class LsstCamBaseMapper(CameraMapper):
camera : `lsst.afw.cameraGeom.Camera`
Camera geometry.
"""
+ return cls._makeYamlCamera(cameraYamlFile=cameraYamlFile)
+ @classmethod
+ @lru_cache(maxsize=10)
+ def _makeYamlCamera(cls, cameraYamlFile=None):
+ """Helper function for _makeCamera that can be cached.
+ """
if not cameraYamlFile:
cameraYamlFile = os.path.join(utils.getPackageDir(cls.packageName), "policy",
("%s.yaml" % cls.getCameraName()))
~
The new method has to exist because policy is not hashable but it's not used anyhow. This means I can instantiate later mappers almost instantly.
The gen2 mapper associated with a gen2 repo maps to a single gen3 instrument. Rather than asking the caller to specify the gen3 instrument explicitly, ask the mapper to tell us. This removes the config entry for instrument and the command line override. It does currently require an entire Butler instance to be created solely to read the mapper class. This may be slow.
In obs_subaru the gen2mapper can not easily import the HyperSuprimeCam class. Therefore we set a string and convert to class on first access.
No description provided.