Implement number of islands #801
Conversation
* added number_of_islands to image/cleaning.py (Calculates the number of connected clusters on a pixel mask) * added unit test with LSTCam geometry
Codecov Report
@@ Coverage Diff @@
## master #801 +/- ##
==========================================
+ Coverage 71.9% 71.96% +0.05%
==========================================
Files 204 204
Lines 11023 11046 +23
==========================================
+ Hits 7926 7949 +23
Misses 3097 3097
Continue to review full report at Codecov.
|
ctapipe/image/cleaning.py
Outdated
| """ | ||
| hits = np.where(mask)[0] | ||
| # store information about marked pixels to avoid double counting | ||
| marked = np.ones(hits.shape[0]) * -1 |
There was a problem hiding this comment.
this could be a python set or a boolean mask of length n_pixels. I don't think an integer array with the indices is the best choice here.
ctapipe/image/cleaning.py
Outdated
| i = 0 | ||
| count = 0 | ||
|
|
||
| for hit in hits: |
There was a problem hiding this comment.
I'm not sure this works for cluster with a radius > 2. You need to treat the neighbors of the neighbors recursively for that.
|
When I worked on this bigger clusters were counted correctly. In hindsight this might need the pixels to follow a specific camera geometry tho. |
LukasNickel
force-pushed
the
implement_number_of_islands
branch
3 times, most recently
from
65c4f44
to
36e298b
Compare
|
This is written in a very non-pythonic way, so it likely quite inefficient, though perhaps it could be sped up quite a bit with a numba I suspect there is already code to count islands somewhere in the scientific python stack, since we use this technique already. @jjlk do you use a better method for this? |
|
Yes, this works perfectly. @LukasNickel can you change the code to use the scipy function? |
That looks pretty good. I didn't expect a scipy function for an arbitrary geometry, but sparse graphs seems to be the keyword here. I'll play around with that function and upload a cleaner scipy version later |
- returns integer array of size mask.n_pixels now aswell -> allows seperating the clusters - cluster sizes > 2 should be counted correctly now - unit test contains a bigger cluster for LSTCam geometry
LukasNickel
force-pushed
the
implement_number_of_islands
branch
from
36e298b
to
f4bacd8
Compare
|
Looks good! |
|
@maxnoe do you still want changes? If not, you can accept your review and I'll merge this, |
* master: (60 commits) Add test that shows slicing breaks cam geom and fix it (cta-observatory#782) fix ctapipe build failure (cta-observatory#811) fix package name for yaml (should be pyyaml) (cta-observatory#810) Implement number of islands (cta-observatory#801) fixed ranges of cam-display so they correspond to fixed toymodel sims (cta-observatory#808) Fix unknown section example warning (cta-observatory#800) Fix timing parameters for case when there are negative values in image (cta-observatory#804) Update Timing Parameters (cta-observatory#799) speed up unit tests that use test_event fixture (cta-observatory#798) Add unit to h_max in HillasReconstructor (cta-observatory#797) Codacy code style improvements (cta-observatory#796) Minor changes: mostly deprecationwarning fixes (cta-observatory#787) Array plotting (cta-observatory#784) added a config file for github change-drafter plugin (cta-observatory#795) Simple HESS adaptations (cta-observatory#794) add test for sliced geometries for hillas calculation (cta-observatory#781) Impact intersection (cta-observatory#778) updated main documentation page (cta-observatory#792) Implement concentration image features (cta-observatory#791) Fix bad builds by changing channel name (missing pyqt package) (cta-observatory#793) ... # Conflicts: # ctapipe/calib/camera/dl1.py
(Calculates the number of connected clusters on a pixel mask)