Skip to content

add min_dist_bad_bin argument for calculate_insulation_score #266

Closed

Description

@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?

Activity

  1. gfudenberg commented on Nov 4, 2021

    @gfudenberg
    Member

    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

  2. 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
  3. added a commit that references this issue on Nov 10, 2021
  4. gfudenberg commented on Nov 12, 2021

    @gfudenberg
    Member

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

  5. agalitsyna commented on Nov 13, 2021

    @agalitsyna
    Member

    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?

  6. gfudenberg commented on Nov 13, 2021

    @gfudenberg
    Member
  7. agalitsyna commented on Nov 13, 2021

    @agalitsyna
    Member

    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

  8. gfudenberg commented on Nov 17, 2021

    @gfudenberg
    Member

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

  9. golobor commented on Nov 18, 2021

    @golobor
    Member

    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.

  10. Phlya commented on Nov 18, 2021

    @Phlya
    Member

    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?

  11. gfudenberg commented on Dec 13, 2021

    @gfudenberg
    Member

    Should we close this / move to discussion ?

  12. locked and limited conversation to collaborators on Dec 13, 2021
  13. converted this issue into a discussion #321 on Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions