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-30892: add BPS configuration snippets #31

Merged
merged 4 commits into from Sep 28, 2022
Merged

Conversation

TallJimbo
Copy link
Member

No description provided.

@TallJimbo TallJimbo force-pushed the tickets/DM-30892 branch 2 times, most recently from 622a4a9 to 19a5ab4 Compare July 14, 2022 20:30
# Use it by adding:
#
# includeConfigs:
# - ${DRP_PIPE_DIR}/bps/clustering/DRP-recalibrated.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this line be indented by one more whitespace? I.e., so that the - is directly below the c in includeConfigs?

# (with no outer indentation) to your BPS config file.

clusterAlgorithm: lsst.ctrl.bps.quantum_clustering_funcs.dimension_clustering
cluster:
Copy link
Contributor

Choose a reason for hiding this comment

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

General comment: starting here, it seems like the rest of this file is using a tab indent of 4 spaces, whereas I think this needs to be 2 spaces (as with the other YAMLs in this PR).

Copy link
Member

Choose a reason for hiding this comment

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

The yaml linter configuration just requires that we be consistent within a single file. Maybe we need to be a bit more draconian.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've just switched this file to 2-space indents for now.

# Use it by adding:
#
# includeConfigs:
# - ${DRP_PIPE_DIR}/bps/resources/HSC/DRP-RC2.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Another indent required here too.

requestMemory: 10000
consolidateForcedSourceOnDiaObjectTable:
requestMemory: 490000
queue: "SLAC_Rubin_Merge"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this (queue: ...) live here in this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch!

# Use it by adding:
#
# includeConfigs:
# - ${DRP_PIPE_DIR}/bps/resources/LSSTCam-imSim/DRP-DP0.2.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

An extra indent is needed here too.

# Use it by adding:
#
# includeConfigs:
# - ${DRP_PIPE_DIR}/bps/resources/LSSTCam-imSim/DRP-test-med-1.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

And one final indent is needed here too.

TallJimbo and others added 4 commits September 27, 2022 15:55
These are taken from three different github repos (all lsst-dm):

  dp02-processing/blob/main/full/production/step1/requestMemory.yaml

  gen3_bps_dc2/tree/main/weekly_templates/*.yaml

  gen3_bps_rc2/tree/main/weekly_templates/*.yaml

I very much doubt they're complete, except perhaps the DP0.2 one, as
the HSC/RC2 and DC2/test-med-1 each provide some values the other does
not.
@TallJimbo TallJimbo merged commit 414ee58 into main Sep 28, 2022
@TallJimbo TallJimbo deleted the tickets/DM-30892 branch September 28, 2022 13:56
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

5 participants