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-37417: Remove random ordering #4

Merged
merged 1 commit into from Dec 21, 2022
Merged

DM-37417: Remove random ordering #4

merged 1 commit into from Dec 21, 2022

Conversation

cmsaunders
Copy link
Collaborator

No description provided.

Copy link
Contributor

@erykoff erykoff left a comment

Choose a reason for hiding this comment

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

Some minor wording comments and some stray debugging that needs to be removed.

@@ -345,6 +346,11 @@ class GbdesAstrometricFitConfig(pipeBase.PipelineTaskConfig,
doc="Exclude reference objects without proper motion/parallax information.",
default=True
)
fitReserveRandomSeed = pexConfig.Field(
dtype=int,
doc="Set the random seed for selecting data points to reserve from the fit for validate.",
Copy link
Contributor

Choose a reason for hiding this comment

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

for "validation"?

inputVisitSummary : `list` of `lsst.afw.table.ExposureCatalog`
List of catalogs with per-detector summary information.
inputVisitSummary : `lsst.afw.table.ExposureCatalog`
Catalogs with per-detector summary information.
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 be single Catalog?

@@ -967,7 +979,7 @@ def _add_objects(self, wcsf, inputCatalogRefs, sourceIndices, extensionInfo, col
for inputCatalogRef in inputCatalogRefs:
visit = inputCatalogRef.dataId['visit']
inputCatalog = inputCatalogRef.get(parameters={'columns': columns})
detectors = set(inputCatalog['detector'])
detectors = np.unique(inputCatalog['detector'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment that np.unique will output a sorted ordering of detector?

@@ -500,6 +504,8 @@ def test_run(self):
fitYPoly = fitModel[len(origXPoly):]
absDiffX = abs(fitXPoly - origXPoly)
absDiffY = abs(fitYPoly - origYPoly)
print(absDiffX)
print(absDiffY)
Copy link
Contributor

Choose a reason for hiding this comment

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

These print statements should go.

@cmsaunders cmsaunders changed the title DM-37417: Remove places with random ordering DM-37417: Remove random ordering Dec 21, 2022
This also sets the random number seed in the tests to ensure
repeatability.
@cmsaunders cmsaunders merged commit a1a777d into main Dec 21, 2022
@cmsaunders cmsaunders deleted the tickets/DM-37417 branch December 21, 2022 02:37
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

2 participants