Skip to content
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-23975: Convert subfilter type to int #242

Merged
merged 2 commits into from Mar 28, 2020
Merged

DM-23975: Convert subfilter type to int #242

merged 2 commits into from Mar 28, 2020

Conversation

isullivan
Copy link
Contributor

abstract_filter is now a required dependency of subfilter
subfilter is now an int

@@ -75,11 +75,14 @@ dimensions:
model wavelength-dependent effects such as differential chromatic
refraction.
keys:
-
name: id
type: int
-
name: name
Copy link
Member

Choose a reason for hiding this comment

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

Are you still going to use the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed it was still necessary, since other dimensions that defined an id also defined a name. I'm not aware of any code that specifically uses the name of a subfilter.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my test running ci_hsc_gen3 with these changes followed by the Gen 3 version of DcrAssembleCoaddTask, I found this works if I leave the dependency on abstract_filter as implied, but fails if I change it to required. From our Slack conversation, it sounded like required would be preferred, but I do not know what I need to change to make that work.

The error message is:

pipetask run -j 1 -b ci_hsc_gen3/DATA/butler.yaml -i shared/ci_hsc_output -o gen3_butler_test1 --register-dataset-types -p dcrAssembleCoadd.yaml -d "detector = 22 AND visit = 903334" --clobber-output
Failed to build graph: "No value in data ID ({Dimension(instrument): 'HSC', Dimension(skymap): 'discrete/ci_hsc', Dimension(detector): 22, Dimension(subfilter): 0, Dimension(tract): 0, Dimension(patch): 69, Dimension(visit): 903334}) for required dimension 'abstract_filter'."
Traceback (most recent call last):
  File "/project/sullivan/code/daf_butler/python/lsst/daf/butler/core/dimensions/coordinate.py", line 155, in standardize
    values = tuple(d[name] for name in graph.required.names)
  File "/project/sullivan/code/daf_butler/python/lsst/daf/butler/core/dimensions/coordinate.py", line 155, in <genexpr>
    values = tuple(d[name] for name in graph.required.names)
KeyError: 'abstract_filter'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/software/lsstsw/stack_20200220/stack/miniconda3-4.7.12-984c9f7/Linux64/ctrl_mpexec/19.0.0-9-g3f34418+9/bin/pipetask", line 26, in <module>
    sys.exit(CmdLineFwk().parseAndRun())
  File "/software/lsstsw/stack_20200220/stack/miniconda3-4.7.12-984c9f7/Linux64/ctrl_mpexec/19.0.0-9-g3f34418+9/python/lsst/ctrl/mpexec/cmdLineFwk.py", line 156, in parseAndRun
    qgraph = self.makeGraph(pipeline, args)
  File "/software/lsstsw/stack_20200220/stack/miniconda3-4.7.12-984c9f7/Linux64/ctrl_mpexec/19.0.0-9-g3f34418+9/python/lsst/ctrl/mpexec/cmdLineFwk.py", line 340, in makeGraph
    qgraph = graphBuilder.makeGraph(pipeline, inputCollections, outputCollection, args.data_query)
  File "/software/lsstsw/stack_20200220/stack/miniconda3-4.7.12-984c9f7/Linux64/pipe_base/19.0.0-13-gafd90fe+1/python/lsst/pipe/base/graphBuilder.py", line 793, in makeGraph
    scaffolding.fillDataIds(self.registry, inputCollections, userQuery)
  File "/software/lsstsw/stack_20200220/stack/miniconda3-4.7.12-984c9f7/Linux64/pipe_base/19.0.0-13-gafd90fe+1/python/lsst/pipe/base/graphBuilder.py", line 516, in fillDataIds
    for commonDataId in resultIter:
  File "/project/sullivan/code/daf_butler/python/lsst/daf/butler/registry/_registry.py", line 1196, in queryDimensions
    yield self.expandDataId(result, records=standardizedDataId.records)
  File "/project/sullivan/code/daf_butler/python/lsst/daf/butler/registry/_registry.py", line 1065, in expandDataId
    record = storage.fetch(keys)
  File "/project/sullivan/code/daf_butler/python/lsst/daf/butler/registry/dimensions/caching.py", line 84, in fetch
    dataId = DataCoordinate.standardize(dataId, graph=self.element.graph)
  File "/project/sullivan/code/daf_butler/python/lsst/daf/butler/core/dimensions/coordinate.py", line 157, in standardize
    raise KeyError(f"No value in data ID ({mapping}) for required dimension {err}.") from err
KeyError: "No value in data ID ({Dimension(instrument): 'HSC', Dimension(skymap): 'discrete/ci_hsc', Dimension(detector): 22, Dimension(subfilter): 0, Dimension(tract): 0, Dimension(patch): 69, Dimension(visit): 903334}) for required dimension 'abstract_filter'."

Copy link
Member

Choose a reason for hiding this comment

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

I think this must be a bug in the query code. I'll try to reproduce it tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's on lsst-dev under /project/sullivan/gen3_hsc_test4/ and the contents are:

description: "Test running dcrAssembleCoadd as a PipelineTask"

instrument: lsst.obs.subaru.gen3.hsc.instrument.HyperSuprimeCam

tasks:
  warp:
    class: lsst.pipe.tasks.makeCoaddTempExp.MakeWarpTask
  cw:
    class: lsst.pipe.tasks.assembleCoadd.CompareWarpAssembleCoaddTask
  dcr:
    class: lsst.pipe.tasks.dcrAssembleCoadd.DcrAssembleCoaddTask

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I call it with:
pipetask run -j 1 -b ci_hsc_gen3/DATA/butler.yaml -i shared/ci_hsc_output -o gen3_butler_test1 --register-dataset-types -p dcrAssembleCoadd.yaml -d "detector = 22 AND visit = 903334" --clobber-output

Copy link
Member

Choose a reason for hiding this comment

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

BTW Today I merged a ticket changing the instrument name. It's now lsst.obs.subaru.HyperSuprimeCam.

Copy link
Member

Choose a reason for hiding this comment

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

I've just pushed a commit to daf_butler that should fix the problem; it was indeed a bug in the query logic (this is the first time we've had one dimension that requires another dimension that is implied by a third dimension). That let me get through QG generation with your how-to-reproduce instructions, until a later failure in the task code itself about filter minimum and maximum wavelengths not being set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect! That later failure is because it needs a ticket branch of obs_subaru that defines a placeholder filter wavelength range, but I will double-check that everything really does work.

@isullivan isullivan force-pushed the tickets/DM-23975 branch 2 times, most recently from d2ec93b to f8ebb3d Compare March 27, 2020 19:12
This fixes the logic to correctly handle the case where one
dimension's required dependency is another dimension's implied
dependency.  It also moves it from a private attribute computed at
construction to a construct-on-first-use public property.  It probably
should have always been public (it's used exclusively by Registry),
and it now needs to be construct-on-first-use to avoid cycles during
DimensionUniverse construction.
@isullivan isullivan merged commit 64a679d into master Mar 28, 2020
@timj timj deleted the tickets/DM-23975 branch April 22, 2020 22:01
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.

None yet

3 participants