Skip to content
This repository was archived by the owner on Sep 11, 2023. It is now read-only.

Conversation

@peterdudfield
Copy link
Contributor

@peterdudfield peterdudfield commented Nov 12, 2021

Pull Request

Description

  • use multiprocess to get spatial locations for examples

Fixes #304

How Has This Been Tested?

normal unittests, which do cover this function

  • No
  • Yes

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@peterdudfield peterdudfield marked this pull request as ready for review November 12, 2021 16:42
@peterdudfield peterdudfield marked this pull request as draft November 12, 2021 17:19
@peterdudfield
Copy link
Contributor Author

Move dback to a draft as #375 might be better

@JackKelly JackKelly added the enhancement New feature or request label Nov 16, 2021
@JackKelly
Copy link
Contributor

I've skim-read the code and this looks good but I agree, let's work on the assumption that #375 will be sufficient 🙂 If #375 isn't sufficient then let's revisit parellising Manager.sample_spatial_and_temporal_locations_for_examples(). Is that what you had in mind?

@peterdudfield
Copy link
Contributor Author

I've skim-read the code and this looks good but I agree, let's work on the assumption that #375 will be sufficient 🙂 If #375 isn't sufficient then let's revisit parellising Manager.sample_spatial_and_temporal_locations_for_examples(). Is that what you had in mind?

Yea, hoepfully #375 will be enough, if not can revisit this.
Perhaps I could close this PR for now, so that its not confusing

@JackKelly
Copy link
Contributor

Perhaps I could close this PR for now, so that its not confusing

Sounds good! (But thank you anyway for writing this PR; it does look great!)

@peterdudfield peterdudfield deleted the issue/304-mutli-process-locations branch December 22, 2021 15:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Speed up Manager.sample_spatial_and_temporal_locations_for_examples() by splitting shuffled_t0_datetimes across multliple processes

3 participants