DM-19616: Make IngestIndexReferenceObjectsTask multiprocessing capable #168
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
parejkoj
force-pushed
the
tickets/DM-19616
branch
2 times, most recently
from
24e728e
to
7275c8b
Compare
czwa
reviewed
Jun 20, 2019
| center = lsst.geom.SpherePoint(data[i][config.ra_name], data[i][config.dec_name], lsst.geom.degrees) | ||
| radius = 1*lsst.geom.arcseconds # don't need to search far to get an exact match! | ||
| refCat = refObjLoader.loadSkyCircle(center, radius, filterName).refCat | ||
| # assert refCat['id'] == data[i][config.id_name], f"input: {data[i]}\n\noutput: {refCat['id']}" |
There was a problem hiding this comment.
Is this assert form sometimes needed, or can it be removed entirely?
There was a problem hiding this comment.
Left over from an earlier version. Thanks for noticing.
| @classmethod | ||
| def makeSkyCatalog(cls, outPath, size=1000, idStart=1, seed=123): | ||
| """Make an on-sky catalog, and save it to a text file. | ||
|
|
tests/ingestIndexTestBase.py
Outdated
| extra_col3 = np.array([get_word(num) for num in np.random.randint(11, size=size)]) | ||
|
|
||
| dtype = np.dtype([('id', float), ('ra_icrs', float), ('dec_icrs', float), | ||
| ('ra_err', float), ('dec_err', float), ('a', float), |
There was a problem hiding this comment.
Match indent style inside the list of tuples. I'm surprised this didn't complain on build.
There was a problem hiding this comment.
I too am surprised that didn't trigger the linter.
tests/test_htmIndex.py
Outdated
| @@ -362,30 +198,24 @@ def testIngest(self): | |||
| ] | |||
| config2.file_reader.delimiter = '|' | |||
| # this also tests changing the delimiter | |||
| # import os; print(os.getpid()); import ipdb; ipdb.set_trace() | |||
| counter = 0 | ||
| # global shared counter to keep track of number of files processed. | ||
| fileProgress = 0 | ||
|
|
There was a problem hiding this comment.
Per the style guide (https://developer.lsst.io/python/style.html?highlight=module%20variables#style-guide-py-sci-pi-naming), I think these global variables should be all caps.
czwa
approved these changes
Jun 20, 2019
Table.read has a fast csv reader that is ~4x faster than genfromtxt. Take just the numpy array portion of it so it behaves like the old code. Add a format option to config, to support different styles of text file.
This refcator will make it easier to pull out the parallelizable portions, specifically `ingestOneFile()`, while separating the assignment of indices from the rest of the calculations.
Split the parallelized work into a separate module, so that it doesn't deal with pickling a Butler, or DataRefs. Add progress indicator to ingest manager. The previous test code did not check that the ingested catalog actually contained the same things as the input catalog. I've improved the tests to do that, and also pulled out some shared code so that I could write unit tests of parts of IngestIndexedReferenceTask and a separate test of the multiprocessing portion of the code. That new test is run outside of pytest, so that it doesn't interact with the pytest multiprocessing.
Centroid fields do not make sense in loadSkyCircle, but they are needed for loadPixelBox. So only add them to the schema for the latter.
Configs are validated before they are frozen, during Task creation, so this line was pointless.
parejkoj
force-pushed
the
tickets/DM-19616
branch
from
7275c8b
to
95761a5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.