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

Use spawn for all multiprocessing #48

Closed
wants to merge 6 commits into from

Conversation

scottstanie
Copy link
Contributor

I'll try and test this more, but I realized that in dolphin's CI, I hadn't installed spurt to run the optional tests. Once I did, it started hanging indefinitely for anything larger than like 20x20 pixels. I think the reason is some bad combination of the libraries which use threads in earlier tests (see here for long discussion).

When I ran the tests with this branch, the went normally.

TODO: check that this doesn't make the temporal unwrapping part very slow.

@scottstanie scottstanie requested review from gmgunter and piyushrpt and removed request for gmgunter October 14, 2024 02:44
@piyushrpt
Copy link
Member

piyushrpt commented Oct 14, 2024

It would be interesting to see how this goes in the production env. This does seem to slow things down on my OSX machine. The reason that things may have worked fast on tests is that reducing the size of the problems might have eliminated any residues - in which case the parallelized bit of code is not exercised.

@piyushrpt
Copy link
Member

Do we close this PR now that bumping up the max_tiles argument allows us to handle larger datasets?

@scottstanie scottstanie closed this Nov 1, 2024
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.

2 participants