You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
add min_dist_bad_bin argument for calculate_insulation_score #266
Old issue title: Insulation score does not ignore "NaN" regions; additionally, can diamond insulation be normalized to surrounding region?
Hi, there are two separate concerns here:
The insulation score does not seem to ignore regions that are NaN in the coolfile, leading to discrepancies
The diamond insulation score should be normalized to the regions proximal to the diamond, so that you are measuring "insulation compared to background". Is there some program which does this?
min_dist_bad_bin=0,
to find_boundaries, but should add this as an argument with sensible defaults to the calculate_insulation_score. (The current version of calculate_insulation_score assumed the user would use the returned table to filter insulation however they wish for downstream analysis).
The CLI should be updated to pass these to calculate_insulation_score as well.
We should decide, however, if we set min_dist_bad_bin=1 by default (it may be more interpretable) cc @golobor
changed the title [-]Insulation score does not ignore "NaN" regions; additionally, can diamond insulation be normalized to surrounding region?[/-][+]add min_dist_bad_bin argument for calculate_insulation_score[/+]on Nov 4, 2021
insulation calculation of cooltools can now ignore NaNs and neighboring regions, controlled by min_dist_bad_bin param (introduced with pre-0.5.0 updates here 98b11db).
@gfudenberg , what do you think about the normalization of insulation to the background?
Ah, okay, I had another explanation in mind. There is another widespread type of insulation normalization where the IS track is divided not by the whole chromosome average but by the moving centered average. This is not a traditional insulation score definition, and I am not sure who first implemented it. Some examples of "moving average insulation": [1] and [2]. cooltools has the traditional insulation right now, following the original Crane et al. 2016 work
I am not sure at the moment - what's the motivation for this local normalization? I'd suggest that we do some studies before adding such a non-trivial feature to the main code.
Oh missed this. Yes, I agree with Anton - is there a reason to do this? It shouldn't really affect positions of dips of insulation anyway... And whether it would improve anything is not at all obvious to me. Is there a use case where it works better?
Activity
gfudenberg commentedon Nov 4, 2021
Thanks for the note & sorry for the slow reply.
It appears we pass:
to
find_boundaries, but should add this as an argument with sensible defaults to thecalculate_insulation_score. (The current version of calculate_insulation_score assumed the user would use the returned table to filter insulation however they wish for downstream analysis).The CLI should be updated to pass these to
calculate_insulation_scoreas well.We should decide, however, if we set min_dist_bad_bin=1 by default (it may be more interpretable) cc @golobor
[-]Insulation score does not ignore "NaN" regions; additionally, can diamond insulation be normalized to surrounding region?[/-][+]add min_dist_bad_bin argument for calculate_insulation_score[/+]insulation enhancement: min_dist_bad_bin
Pre0.5.0 (#299)
gfudenberg commentedon Nov 12, 2021
is this addressed in new user-facing
insulationfunction ? cc @Phlya @agalitsynaagalitsyna commentedon Nov 13, 2021
insulation calculation of cooltools can now ignore NaNs and neighboring regions, controlled by min_dist_bad_bin param (introduced with pre-0.5.0 updates here 98b11db).
@gfudenberg , what do you think about the normalization of insulation to the background?
gfudenberg commentedon Nov 13, 2021
agalitsyna commentedon Nov 13, 2021
Ah, okay, I had another explanation in mind. There is another widespread type of insulation normalization where the IS track is divided not by the whole chromosome average but by the moving centered average. This is not a traditional insulation score definition, and I am not sure who first implemented it. Some examples of "moving average insulation": [1] and [2]. cooltools has the traditional insulation right now, following the original Crane et al. 2016 work
gfudenberg commentedon Nov 17, 2021
got it-- sure I'd think a local option could be reasonable to add. other thoughts? @agalitsyna @golobor @Phlya
golobor commentedon Nov 18, 2021
I am not sure at the moment - what's the motivation for this local normalization? I'd suggest that we do some studies before adding such a non-trivial feature to the main code.
Phlya commentedon Nov 18, 2021
Oh missed this. Yes, I agree with Anton - is there a reason to do this? It shouldn't really affect positions of dips of insulation anyway... And whether it would improve anything is not at all obvious to me. Is there a use case where it works better?
gfudenberg commentedon Dec 13, 2021
Should we close this / move to discussion ?