Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-15606

Add jointcal config defaults to at least obs_subaru

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_subaru
    • Labels:
      None
    • Story Points:
      3
    • Sprint:
      DRP F18-4
    • Team:
      Data Release Production

      Description

      Add reference catalog defaults to at least obs_subaru to facilitate running jointcal on the HSC biweeklies.

      John Parejko I assigned it to you, but I can just as easily do it if one of you sends me the command to run jointcal on the RC2 dataset so I can test that the configs make it do what we expect.

        Attachments

          Activity

          Hide
          Parejkoj John Parejko added a comment -

          Hsin-Fang Chiang has the scripts for running jointcal on RC2.

          And I don't know what appropriate defaults are: I think the correct thing is to go with whatever Hsin-Fang Chiang did for her recent run. There's no reason jointcal can't use the same reference catalog defaults as single frame processing for both the astrometry and photometry refcats, for now. If we have gaia, we should maybe use that for astrometry (it doesn't have appropriate photometry yet).

          Show
          Parejkoj John Parejko added a comment - Hsin-Fang Chiang has the scripts for running jointcal on RC2. And I don't know what appropriate defaults are: I think the correct thing is to go with whatever Hsin-Fang Chiang did for her recent run. There's no reason jointcal can't use the same reference catalog defaults as single frame processing for both the astrometry and photometry refcats, for now. If we have gaia, we should maybe use that for astrometry (it doesn't have appropriate photometry yet).
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          The config override I used in DM-15521 was 

          config.astrometryRefObjLoader.ref_dataset_name='ps1_pv3_3pi_20170110'
          config.photometryRefObjLoader.ref_dataset_name='ps1_pv3_3pi_20170110'
          config.astrometryRefObjLoader.filterMap={'B': 'g', 'r2': 'r', 'N1010': 'z', 'N816': 'i', 'I': 'i', 'N387': 'g', 'i2': 'i', 'R': 'r', 'N921': 'z', 'N515': 'g', 'V': 'r'}
          config.photometryRefObjLoader.filterMap={'B': 'g', 'r2': 'r', 'N1010': 'z', 'N816': 'i', 'I': 'i', 'N387': 'g', 'i2': 'i', 'R': 'r', 'N921': 'z', 'N515': 'g', 'V': 'r'}
          

          but as said, probably better to be exactly the same as in the single frame processing such as loading filterMap.py like here in obs_subaru.

          We do have /datasets/refcats/htm/gaia_DR1_v1/ on LSST machines. Do you prefer that?

          One command for the test run can be:

          jointcal.py /datasets/hsc/repo --calib /datasets/hsc/repo/CALIB --rerun RC/w_2018_34/DM-15517:private/user/jointcal --id ccd=0..8^10..103 visit=26024^26028^26032^26036^26044^26046^26048^26050^26058^26060^26062^26070^26072^26074^26080^26084^26094 filter=HSC-G tract=9615 
          

          Show
          hchiang2 Hsin-Fang Chiang added a comment - The config override I used in DM-15521 was  config.astrometryRefObjLoader.ref_dataset_name='ps1_pv3_3pi_20170110' config.photometryRefObjLoader.ref_dataset_name='ps1_pv3_3pi_20170110' config.astrometryRefObjLoader.filterMap={'B': 'g', 'r2': 'r', 'N1010': 'z', 'N816': 'i', 'I': 'i', 'N387': 'g', 'i2': 'i', 'R': 'r', 'N921': 'z', 'N515': 'g', 'V': 'r'} config.photometryRefObjLoader.filterMap={'B': 'g', 'r2': 'r', 'N1010': 'z', 'N816': 'i', 'I': 'i', 'N387': 'g', 'i2': 'i', 'R': 'r', 'N921': 'z', 'N515': 'g', 'V': 'r'} but as said, probably better to be exactly the same as in the single frame processing such as loading filterMap.py like here in obs_subaru. We do have /datasets/refcats/htm/gaia_DR1_v1/ on LSST machines. Do you prefer that? One command for the test run can be: jointcal.py /datasets/hsc/repo --calib /datasets/hsc/repo/CALIB --rerun RC/w_2018_34/DM-15517:private/user/jointcal --id ccd=0..8^10..103 visit=26024^26028^26032^26036^26044^26046^26048^26050^26058^26060^26062^26070^26072^26074^26080^26084^26094 filter=HSC-G tract=9615
          Hide
          Parejkoj John Parejko added a comment -

          We do have /datasets/refcats/htm/gaia_DR1_v1/ on LSST machines. Do you prefer that?

          Lets stick with PS1 for now, and see what gaia does as a different test.

          Show
          Parejkoj John Parejko added a comment - We do have /datasets/refcats/htm/gaia_DR1_v1/ on LSST machines. Do you prefer that? Lets stick with PS1 for now, and see what gaia does as a different test.
          Hide
          yusra Yusra AlSayyad added a comment -

          I found ref cats and filterMaps overrides in two other obs_* packages, that we'd conceivably want to run jointcal on: obs_lsstCam, obs_decam.

          Would you review?

          Dominique Boutigny, at a quick glance I couldn't tell which reference catalog is your default in https://github.com/lsst/obs_cfht/blob/master/config/processCcd.py

          Show
          yusra Yusra AlSayyad added a comment - I found ref cats and filterMaps overrides in two other obs_* packages, that we'd conceivably want to run jointcal on: obs_lsstCam, obs_decam. Would you review? Dominique Boutigny , at a quick glance I couldn't tell which reference catalog is your default in https://github.com/lsst/obs_cfht/blob/master/config/processCcd.py
          Hide
          yusra Yusra AlSayyad added a comment -

          Hmm. Needs a little more. 

          The unit tests do:
          self.config.astrometryRefObjLoader.retarget(LoadAstrometryNetObjectsTask)
          but the overrides in the obs* packages get used in the unit tests (via parseAndRun).

          (I understand Dominique's note in obs_cfht now)

          =================================== FAILURES ===================================
           JointcalTestDECAM.test_jointcalTask_2_visits_constrainedAstrometry_no_photometry 
          [gw3] linux -- Python 3.6.6 /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/miniconda/envs/lsst-scipipe/bin/python3.6
           
          self = <test_jointcal_decam.JointcalTestDECAM testMethod=test_jointcalTask_2_visits_constrainedAstrometry_no_photometry>
           
              def test_jointcalTask_2_visits_constrainedAstrometry_no_photometry(self):
                  relative_error, pa1, metrics = self.setup_jointcalTask_2_visits_constrainedAstrometry_no_photometry()
              
          >       self._testJointcalTask(2, relative_error, self.dist_rms_absolute, pa1, metrics=metrics)
           
          tests/test_jointcal_decam.py:109: 
          _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
          tests/jointcalTestBase.py:134: in _testJointcalTask
              result = self._runJointcalTask(nCatalogs, caller, metrics=metrics)
          tests/jointcalTestBase.py:180: in _runJointcalTask
              result = jointcal.JointcalTask.parseAndRun(args=args, doReturnResults=True, config=self.config)
          ../../stack/Linux64/pipe_base/16.0-11-g9fe0e56+9/python/lsst/pipe/base/cmdLineTask.py:598: in parseAndRun
              parsedCmd = argumentParser.parse_args(config=config, args=args, log=log, override=cls.applyOverrides)
          ../../stack/Linux64/pipe_base/16.0-11-g9fe0e56+9/python/lsst/pipe/base/argumentParser.py:634: in parse_args
              self._applyInitialOverrides(namespace)
          ../../stack/Linux64/pipe_base/16.0-11-g9fe0e56+9/python/lsst/pipe/base/argumentParser.py:832: in _applyInitialOverrides
              namespace.config.load(filePath)
          ../../stack/Linux64/pex_config/16.0-3-g9645794+3/python/lsst/pex/config/config.py:543: in load
              self.loadFromStream(stream=code, root=root)
          ../../stack/Linux64/pex_config/16.0-3-g9645794+3/python/lsst/pex/config/config.py:563: in loadFromStream
              exec(stream, {}, local)
          ../../stack/Linux64/obs_decam/tickets.DM-15606-g8e0a9334c4/config/jointcal.py:1: in <module>
              config.astrometryRefObjLoader.ref_dataset_name = 'ps1_pv3_3pi_20170110'
          ../../stack/Linux64/pex_config/16.0-3-g9645794+3/python/lsst/pex/config/configurableField.py:122: in __setattr__
              self._value.__setattr__(name, value, at=at, label=label)
          _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
           
          self = lsst.meas.algorithms.loadReferenceObjects.LoadReferenceObjectsConfig(pixelMargin=300, defaultFilter='', filterMap={})
          attr = 'ref_dataset_name', value = 'ps1_pv3_3pi_20170110'
          at = [StackFrame(<string>, 1, <module>), StackFrame(<string>, 1539, <module>), StackFrame(<string>, 1534, serve), StackFram..., 1047, serve), StackFrame(<string>, 259, integrate_as_primary_thread), StackFrame(<string>, 277, _perform_spawn), ...]
          label = 'assignment'
           
              def __setattr__(self, attr, value, at=None, label="assignment"):
                  """!Regulate which attributes can be set
              
                      Unlike normal python objects, Config objects are locked such
                      that no additional attributes nor properties may be added to them
                      dynamically.
              
                      Although this is not the standard Python behavior, it helps to
                      protect users from accidentally mispelling a field name, or
                      trying to set a non-existent field.
                      """
                  if attr in self._fields:
                      if at is None:
                          at = getCallStack()
                      # This allows Field descriptors to work.
                      self._fields[attr].__set__(self, value, at=at, label=label)
                  elif hasattr(getattr(self.__class__, attr, None), '__set__'):
                      # This allows properties and other non-Field descriptors to work.
                      return object.__setattr__(self, attr, value)
                  elif attr in self.__dict__ or attr in ("_name", "_history", "_storage", "_frozen", "_imports"):
                      # This allows specific private attributes to work.
                      self.__dict__[attr] = value
                  else:
                      # We throw everything else.
          >           raise AttributeError("%s has no attribute %s" % (_typeStr(self), attr))
          E           AttributeError: lsst.meas.algorithms.loadReferenceObjects.LoadReferenceObjectsConfig has no attribute ref_dataset_name
          

          Show
          yusra Yusra AlSayyad added a comment - Hmm. Needs a little more.  The unit tests do: self.config.astrometryRefObjLoader.retarget(LoadAstrometryNetObjectsTask) but the overrides in the obs* packages get used in the unit tests (via parseAndRun). (I understand Dominique's note in obs_cfht now) =================================== FAILURES =================================== JointcalTestDECAM.test_jointcalTask_2_visits_constrainedAstrometry_no_photometry [gw3] linux -- Python 3.6.6 /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/miniconda/envs/lsst-scipipe/bin/python3.6   self = <test_jointcal_decam.JointcalTestDECAM testMethod=test_jointcalTask_2_visits_constrainedAstrometry_no_photometry>   def test_jointcalTask_2_visits_constrainedAstrometry_no_photometry(self): relative_error, pa1, metrics = self.setup_jointcalTask_2_visits_constrainedAstrometry_no_photometry() > self._testJointcalTask(2, relative_error, self.dist_rms_absolute, pa1, metrics=metrics)   tests/test_jointcal_decam.py:109: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ tests/jointcalTestBase.py:134: in _testJointcalTask result = self._runJointcalTask(nCatalogs, caller, metrics=metrics) tests/jointcalTestBase.py:180: in _runJointcalTask result = jointcal.JointcalTask.parseAndRun(args=args, doReturnResults=True, config=self.config) ../../stack/Linux64/pipe_base/16.0-11-g9fe0e56+9/python/lsst/pipe/base/cmdLineTask.py:598: in parseAndRun parsedCmd = argumentParser.parse_args(config=config, args=args, log=log, override=cls.applyOverrides) ../../stack/Linux64/pipe_base/16.0-11-g9fe0e56+9/python/lsst/pipe/base/argumentParser.py:634: in parse_args self._applyInitialOverrides(namespace) ../../stack/Linux64/pipe_base/16.0-11-g9fe0e56+9/python/lsst/pipe/base/argumentParser.py:832: in _applyInitialOverrides namespace.config.load(filePath) ../../stack/Linux64/pex_config/16.0-3-g9645794+3/python/lsst/pex/config/config.py:543: in load self.loadFromStream(stream=code, root=root) ../../stack/Linux64/pex_config/16.0-3-g9645794+3/python/lsst/pex/config/config.py:563: in loadFromStream exec(stream, {}, local) ../../stack/Linux64/obs_decam/tickets.DM-15606-g8e0a9334c4/config/jointcal.py:1: in <module> config.astrometryRefObjLoader.ref_dataset_name = 'ps1_pv3_3pi_20170110' ../../stack/Linux64/pex_config/16.0-3-g9645794+3/python/lsst/pex/config/configurableField.py:122: in __setattr__ self._value.__setattr__(name, value, at=at, label=label) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _   self = lsst.meas.algorithms.loadReferenceObjects.LoadReferenceObjectsConfig(pixelMargin=300, defaultFilter='', filterMap={}) attr = 'ref_dataset_name', value = 'ps1_pv3_3pi_20170110' at = [StackFrame(<string>, 1, <module>), StackFrame(<string>, 1539, <module>), StackFrame(<string>, 1534, serve), StackFram..., 1047, serve), StackFrame(<string>, 259, integrate_as_primary_thread), StackFrame(<string>, 277, _perform_spawn), ...] label = 'assignment'   def __setattr__(self, attr, value, at=None, label="assignment"): """!Regulate which attributes can be set Unlike normal python objects, Config objects are locked such that no additional attributes nor properties may be added to them dynamically. Although this is not the standard Python behavior, it helps to protect users from accidentally mispelling a field name, or trying to set a non-existent field. """ if attr in self._fields: if at is None: at = getCallStack() # This allows Field descriptors to work. self._fields[attr].__set__(self, value, at=at, label=label) elif hasattr(getattr(self.__class__, attr, None), '__set__'): # This allows properties and other non-Field descriptors to work. return object.__setattr__(self, attr, value) elif attr in self.__dict__ or attr in ("_name", "_history", "_storage", "_frozen", "_imports"): # This allows specific private attributes to work. self.__dict__[attr] = value else: # We throw everything else. > raise AttributeError("%s has no attribute %s" % (_typeStr(self), attr)) E AttributeError: lsst.meas.algorithms.loadReferenceObjects.LoadReferenceObjectsConfig has no attribute ref_dataset_name
          Hide
          Parejkoj John Parejko added a comment -

          Chris Morrison [X] is in the process of building indexed refcats for the validation datasets, so I should be be able to switch testdata_jointcal to use those once that's done. That might fix the above problem? Or do you have another suggestion about how to have the unittests not get overridden? They're supplying a Config object, so I'm somewhat surprised that it's getting overridden (though I don't know what the hierarchy is there).

          Show
          Parejkoj John Parejko added a comment - Chris Morrison [X] is in the process of building indexed refcats for the validation datasets, so I should be be able to switch testdata_jointcal to use those once that's done. That might fix the above problem? Or do you have another suggestion about how to have the unittests not get overridden? They're supplying a Config object, so I'm somewhat surprised that it's getting overridden (though I don't know what the hierarchy is there).
          Hide
          Parejkoj John Parejko added a comment -

          Per DM-15713, it looks like we want to go with a 7th order visit polynomial for HSC astrometry. This means adding the following to the hsc jointcal config (modify the comment as you see fit):

          # Testing in DM-15713 found that a 7th order visit polynomial markedly improved the astrometric residuals on HSC.
          self.config.astrometryVisitOrder = 7
          

          Show
          Parejkoj John Parejko added a comment - Per DM-15713 , it looks like we want to go with a 7th order visit polynomial for HSC astrometry. This means adding the following to the hsc jointcal config (modify the comment as you see fit): # Testing in DM-15713 found that a 7th order visit polynomial markedly improved the astrometric residuals on HSC. self.config.astrometryVisitOrder = 7
          Hide
          price Paul Price added a comment -

          The NAOJ team reported once that a 9th order polynomial was necessary to model the distortion in HSC, which turns on hard at the edge of the FoV.

          Show
          price Paul Price added a comment - The NAOJ team reported once that a 9th order polynomial was necessary to model the distortion in HSC, which turns on hard at the edge of the FoV.
          Hide
          Parejkoj John Parejko added a comment -

          The NAOJ team reported once that a 9th order polynomial was necessary to model the distortion in HSC, which turns on hard at the edge of the FoV.

          Given that jointcal is exceeding meas_mosaic on the SRD metrics with both a 5th and 7th order, I'd rather not go to higher order without some further measurements to show its necessary. That can come later.

          Show
          Parejkoj John Parejko added a comment - The NAOJ team reported once that a 9th order polynomial was necessary to model the distortion in HSC, which turns on hard at the edge of the FoV. Given that jointcal is exceeding meas_mosaic on the SRD metrics with both a 5th and 7th order, I'd rather not go to higher order without some further measurements to show its necessary. That can come later.
          Hide
          Parejkoj John Parejko added a comment -

          I'm going to take this over for a bit to see if I can fix the jointcal failures it introduces.

          Show
          Parejkoj John Parejko added a comment - I'm going to take this over for a bit to see if I can fix the jointcal failures it introduces.
          Hide
          Parejkoj John Parejko added a comment - - edited

          Yusra AlSayyad: I came up with a possible solution, implemented for the jointcal decam tests. Can you please take a look and tell me if you'd be happy with this? If so, I'll do it for the other tests too.

          https://github.com/lsst/jointcal/compare/tickets/DM-15606

          It's not great (it turns lines of actual code into long strings), but it means I don't have to re-implement parts of parseAndRun to build up the dataId list, etc.

          Show
          Parejkoj John Parejko added a comment - - edited Yusra AlSayyad : I came up with a possible solution, implemented for the jointcal decam tests. Can you please take a look and tell me if you'd be happy with this? If so, I'll do it for the other tests too. https://github.com/lsst/jointcal/compare/tickets/DM-15606 It's not great (it turns lines of actual code into long strings), but it means I don't have to re-implement parts of parseAndRun to build up the dataId list, etc.
          Hide
          yusra Yusra AlSayyad added a comment -

          A configfile solution is on u/yusra/DM-15606 in jointcal and passes Jenkins (https://ci.lsst.codes/job/stack-os-matrix/28702).  I'll open a PR if you'd be up for reviewing. Before merging, I would put it on tickets/DM-15606 and squash the commits. 

          I added the config.astrometryVisitOrder = 7 to the obs_subaru PR too.

          Show
          yusra Yusra AlSayyad added a comment - A configfile solution is on u/yusra/ DM-15606 in jointcal and passes Jenkins ( https://ci.lsst.codes/job/stack-os-matrix/28702 ).  I'll open a PR if you'd be up for reviewing. Before merging, I would put it on tickets/ DM-15606  and squash the commits.  I added the config.astrometryVisitOrder = 7 to the obs_subaru PR too.
          Hide
          Parejkoj John Parejko added a comment -

          Quick skim suggests this is much better than my attempt. I'd be happy to review, and will expedite it: rename the ticket branch now (feel free to wipe out my branch above) and do whatever squashing you want, and I'll review the PR asap.

          Show
          Parejkoj John Parejko added a comment - Quick skim suggests this is much better than my attempt. I'd be happy to review, and will expedite it: rename the ticket branch now (feel free to wipe out my branch above) and do whatever squashing you want, and I'll review the PR asap.
          Hide
          yusra Yusra AlSayyad added a comment -

          Commits squashed, PR opened, and https://ci.lsst.codes/job/stack-os-matrix/28714 succeeded.

          Show
          yusra Yusra AlSayyad added a comment - Commits squashed, PR opened, and https://ci.lsst.codes/job/stack-os-matrix/28714  succeeded.
          Hide
          Parejkoj John Parejko added a comment -

          One tiny comment, otherwise this looks very good. Thanks for coming up with it!

          Show
          Parejkoj John Parejko added a comment - One tiny comment, otherwise this looks very good. Thanks for coming up with it!
          Hide
          yusra Yusra AlSayyad added a comment -

          Thank you John Parejko

          Show
          yusra Yusra AlSayyad added a comment - Thank you John Parejko

            People

            Assignee:
            yusra Yusra AlSayyad
            Reporter:
            yusra Yusra AlSayyad
            Reviewers:
            John Parejko
            Watchers:
            Hsin-Fang Chiang, John Parejko, Paul Price, Yusra AlSayyad
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.