Optimize memory usage of pycbc_fit_sngls_split_binned #4543
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
spxiwh
added
the
v23_release_branch
There was a problem hiding this comment.
This all looks sensible to me, a couple of minor comments
Index juggling is always a bit of a headache, but I've tried to follow it through and it looks right. The proof is in the pudding through so if you could add some before / after plots that would be great
| mask_start_idx = np.zeros_like(boundaries) | ||
| mask_end_idx = np.zeros_like(boundaries) | ||
| for idx_start in sorted_boundary_list: | ||
| boundary_idx = np.argmax(boundaries == idx_start) |
There was a problem hiding this comment.
Would you be able to do an argsort on boundaries above, and then translate between the two orders more easily?
tdent
reviewed
Oct 30, 2023
tdent
reviewed
Oct 30, 2023
|
@spxiwh have you pushed the actual changes to the PR branch? |
|
No, sorry, had a bunch of students show up in the middle of responding. I also noticed that the tests failed, so am following up on that ... will post when branch is updated. |
|
I've pushed the comment changes. I understand the test failure (it will fail again), but need to check with @GarethCabournDavies what to do here. |
|
From my perspective this should be good now (if tests pass). The example doesn't actually test this code at present ... I'll leave turning that on to a different PR. |
tdent
reviewed
Oct 31, 2023
tdent
reviewed
Oct 31, 2023
Co-authored-by: Thomas Dent <[email protected]>
tdent
approved these changes
Oct 31, 2023
|
Comment merged and question replied to! Thanks. |
spxiwh
added a commit
to spxiwh/pycbc
that referenced
this pull request
Nov 2, 2023
* Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]>
spxiwh
added a commit
that referenced
this pull request
Nov 3, 2023
* Read DQ flags with finer time resolution (#4547) * Read DQ flags with finer time resolution * No default sample rate for dq flags * No default sample rate for dq * Add type for frequency command line option * Optimize memory usage of pycbc_fit_sngls_split_binned (#4543) * Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]> * Non-coinc ifos in minifollowup trig plots (#4548) * Update get.sh (#4532) --------- Co-authored-by: maxtrevor <[email protected]> Co-authored-by: Thomas Dent <[email protected]> Co-authored-by: Shichao Wu <[email protected]>
PRAVEEN-mnl
pushed a commit
to PRAVEEN-mnl/pycbc
that referenced
this pull request
Nov 3, 2023
* Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]>
spxiwh
removed
the
v23_release_branch
maxtrevor
pushed a commit
to maxtrevor/pycbc
that referenced
this pull request
Dec 11, 2023
* Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]>
bhooshan-gadre
pushed a commit
to bhooshan-gadre/pycbc
that referenced
this pull request
Dec 19, 2023
* Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]>
bhooshan-gadre
pushed a commit
to bhooshan-gadre/pycbc
that referenced
this pull request
Mar 4, 2024
* Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]>
lpathak97
pushed a commit
to lpathak97/pycbc
that referenced
this pull request
Mar 13, 2024
* Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]>
acorreia61201
pushed a commit
to acorreia61201/pycbc
that referenced
this pull request
Apr 4, 2024
* Optimize memory usage of pycbc_fit_sngls_split_binned * Need add option here * Comments on PR * Example doesn't use code being edited! * Update bin/all_sky_search/pycbc_fit_sngls_split_binned Co-authored-by: Thomas Dent <[email protected]> --------- Co-authored-by: Thomas Dent <[email protected]>
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.
pycbc_fit_sngls_split_binnedcan be a real issue with memory usage. A recent example at production scale reported 200GB of RAM usage. If we decide that we can ignore triggers with single ranking less than (say) 5.5 this can be greatly reduced .... and if you want all the triggers, you can still use the 200GB!This optimizes the memory usage based on how many triggers are needed. This is now based on
stat-thresholdand the new optionplot-lower-stat-limit. If we could solve #4524 it would simplify the implementation here. I've tested this on a large example and the output plots are indistinguishable to me ... They wouldn't be if I'd messed up the new "idx_map" thing.