Fix plot_singles and optimize memory #4566
Conversation
spxiwh
added
the
v23_release_branch
|
Since mpl 3.8 seems to finally have implemented the 'don't try to reduce cells with 0 points in' default, we should be able to reduce the complexity considerably and only keep the Also I would rather use |
|
Hopefully this addresses the feedback now? |
|
Did you try using np.max instead of plain max ? |
|
Missed that, sorry, will try. |
| data_mask = np.zeros(n_triggers_orig, dtype=bool) | ||
| data_mask[idx] = True | ||
|
|
||
| filts = [f for f in [opts.filter_string] if f is not None] |
There was a problem hiding this comment.
Is the square bracket surrounding opts.filter_string intended here? This line does not make much sense to me (but I am quite tired right now…)
There was a problem hiding this comment.
This appears the same as the existing code and join works with a (non-empty) list.
There was a problem hiding this comment.
(could be a tuple as well, no difference to the outcome)
There was a problem hiding this comment.
The code does work, but I think having moved the SNR mask, the following two lines:
filts = [f for f in [opts.filter_string] if f is not None]
filter_func = ' & '.join(filts) if filts else None
basically reduce to:
filter_func = opts.filter_string
I'll just remove these lines and send opts.filter_string directly to the relevant function call. Then I'll merge unless there are objections.
There was a problem hiding this comment.
ah, I missed the point that this previously added the SNR cut to the filter option and now doesn't ..
|
Seems to work fine with np.max, updated to that. |
|
It's still approved (assuming it continues to work fine) |
* Fix plot_singles and optimize memory * Tom's comment * Use np.max * Remove redundant lines
* Fix plot_singles and optimize memory * Tom's comment * Use np.max * Remove redundant lines
* Fix plot_singles and optimize memory * Tom's comment * Use np.max * Remove redundant lines
spxiwh
removed
the
v23_release_branch
* Fix plot_singles and optimize memory * Tom's comment * Use np.max * Remove redundant lines
* Fix plot_singles and optimize memory * Tom's comment * Use np.max * Remove redundant lines
* Fix plot_singles and optimize memory * Tom's comment * Use np.max * Remove redundant lines
pycbc_plot_singles_vs_paramswas broken by recent changes in matplotlib. The note about "behaviour of hexbin" here https://matplotlib.org/stable/api/prev_api_changes/api_changes_3.8.1.html specifically broke thinks. My understanding is that before we setmincnt = 0, but that actually meant "minimum count is 1 trigger in that bin". Now it means "minimum count is 0 triggers in that bin", and the code fails if you try to take the maximum of 0 triggers.I put in a version-dependent switch on the
mincntparameter to fix this. This resolved the problem in a single test case, I'm running a bunch more in a full workflow now..... It also took ages to debug this, because the code reads way more triggers than it needs to. I've implemented the HFile mask thing in this code as well. This makes the code run significantly faster, and with much less memory usage. The
filter_funcstill uses a lot of memory, but given that we are considering #4524 #4545 I leave that to be made nicer when that change goes in.