add min_dist_bad_bin argument for calculate_insulation_score

[gdolsten]

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:

1. The insulation score does not seem to ignore regions that are NaN in the coolfile, leading to discrepancies
2. 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?

[gfudenberg]

Thanks for the note & sorry for the slow reply.

It appears we pass:

• min_frac_valid_pixels=0.66,
• 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

[gfudenberg]

is this addressed in new user-facing insulation function ? cc @Phlya @agalitsyna

[agalitsyna]

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]

Do you mean the `norm_by_median` argument in insul_diamond that is not passed along to the user-facing function ?

…

On Fri, Nov 12, 2021, 7:51 PM agalitsyna ***@***.***> wrote: 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 <98b11db> ). @gfudenberg <https://github.com/gfudenberg> , what do you think about the normalization to the background? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#266 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEV7GZPZTMCRTL47RLWKJQTULXOFRANCNFSM5DFRXS4A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[agalitsyna]

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]

got it-- sure I'd think a local option could be reasonable to add. other thoughts? @agalitsyna @golobor @Phlya

[golobor]

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]

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]

Should we close this / move to discussion ?