search

PouletAxel / SIP

SIP: Significant Interaction Peak caller
GNU General Public License v3.0
13 stars 3 forks source link

Support for SCALE normalization #16

Closed ksmetz closed 2 years ago

ksmetz commented 2 years ago

Our lab uses and loves SIP for loop calling. However, we recently had to switch from KR to SCALE normalization (KR was not converging in our data), and we have been having some trouble getting SIP to run with SCALE on the command line.

While our juicer_tools version (1.22.01) accepts SCALE as a valid normalization for dump, running SIP with -norm SCALE returns an immediate error of -norm = SCALE, not defined. I believe this is due to lines 415-416 in Hic_main.java, which require -norm to be either NONE, VC, VC_SQRT, or KR.

We have been trying to figure out a workaround using SIP in processed mode and dumping the data ourselves, but we have concerns about faithfully recreating the processing that SIP does by default. It is very likely it would be more involved than this, but in case adding SCALE support would only require including || args[i+1].equals("SCALE") in those lines in Hic_main.java, would it be possible to add support for SCALE in hic mode?

Thanks for your time and help, and for a great tool!

PouletAxel commented 2 years ago

Thanks, for using SIP. I will test your solution. I think it is the good one, after I need to check the values gave by the SCALE parameters and how it impacts the rest of the processing. I will try to give a new version before the week end if everything is going well.

PouletAxel commented 2 years ago

I made the modification, but I can't find a hic file with the SCALE normalization. Could you send me your dataset to test SIP please? Axel

ksmetz commented 2 years ago

Thank you so much for looking into this so quickly! I have a lightly sequenced .hic file (~300-400M reads) that is only 2.5GB and has both KR and SCALE normalization. I will invite you to that Dropbox folder and it should finish uploading within the hour. Please let me know if you have any issues accessing it or anything!

PouletAxel commented 2 years ago

Great Thanks, I will run the test as soon as I have the data

PouletAxel commented 2 years ago

I uploaded in the dropbox the new version I made to correct the SCALE normalization. I used the last version of juicertools : https://github.com/aidenlab/Juicebox/releases. I tested and it worked but before to do a release I would prefer you test it if it is possible.

ksmetz commented 2 years ago

Amazing, thank you! I will test it out with our larger .hic file and report back shortly

ksmetz commented 2 years ago

Hi Axel - it looks like it worked!

One slight caviat I thought I should mention - we have been using a few different versions of juicer_tools as new versions come out, latest of which was version 1.22.01. This was the version we used when building our .hic files and adding norms. However, 1.22 doesn't seem to work with SIP even using KR (the new SIP version you sent OR the current version). It seems to fail while dumping, creating only the first chunk of processed files for most chromosomes. The next-most-recent juicer_tools, version 1.19.02 (and as far as I can tell, every other version) does still seem to work perfectly though.

I don't know much about it, but based on the warning about 1.22 with SIPMeta I imagine it may be a similar case. We can use the other juicer_tools versions for SIP and it works perfectly for our needs, but in case this info is helpful I thought I would share (and the error log below). Thanks again for all your help!

SIPkrTest22-33062263.txt

PouletAxel commented 2 years ago

I know it is not working with the 1.22.01, because the dump expected vector gives only the first value and I don't understand why. I tried the 1.22.01 juicer version on different datasets and each time I obtained only the first value for the expect vector that I am using to compute the value for the image. https://github.com/aidenlab/Juicebox/releases => here you have the juicertools version 2.13 which is working with SIP and should work with SIPMeta. I will make the correction during the weekend for metaSIP and the SCALE normalization.

On Fri, 12 Nov 2021 at 19:22, Kathleen S Metz Reed @.***> wrote:

Hi Axel - it looks like it worked!

One slight caviat I thought I should mention - we have been using a few different versions of juicer_tools as new versions come out, latest of which was version 1.22.01. This was the version we used when building our .hic files and adding norms. However, 1.22 doesn't seem to work with SIP even using KR (the new SIP version you sent OR the current version). It seems to fail while dumping, creating only the first chunk of processed files for most chromosomes. The next-most-recent juicer_tools, version 1.19.02 (and as far as I can tell, every other version) does still seem to work perfectly though.

I don't know much about it, but based on the warning about 1.22 with SIPMeta I imagine it may be a similar case. We can use the other juicer_tools versions for SIP and it works perfectly for our needs, but in case this info is helpful I thought I would share (and the error log below). Thanks again for all your help!

SIPkrTest22-33062263.txt https://github.com/PouletAxel/SIP/files/7531015/SIPkrTest22-33062263.txt

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/PouletAxel/SIP/issues/16#issuecomment-967741314, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABS4UBNTZCFOQ4ULNOT47LLULWVSTANCNFSM5H26BBCA . 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.

ksmetz commented 2 years ago

Hi Axel - Sorry to be so slow to respond! Thanks for pointing out that newer version of juicer_tools. I updated to 2.13 and confirmed that the SIP snapshot you sent is definitely still working on our larger data. We'll use that for everything going forward!

Thanks again for looking into this so quickly! Really appreciate it

ksmetz commented 2 years ago

Hi Axel - hope you're doing well! I saw the source code update for SCALE normalization, but I wasn't sure if there was a jar update or if we should compile it ourselves.

Our lab has little/no experience working with Java and compiling jars, so I was wondering if you were also planning to have a new jar release, or if you might be willing to share a jar with those changes incorporated? Or should we use snapshot jar that you sent before?

Thanks for all your time and help!

PouletAxel commented 2 years ago

I will make a release today with this modification Best Axel Sent from my iPhone

On Nov 29, 2021, at 11:35 AM, Kathleen S Metz Reed @.***> wrote:

 Hi Axel - hope you're doing well! I saw the source code update for SCALE normalization, but I wasn't sure if there was a jar update or if we should compile it ourselves.

Our lab has little/no experience working with Java and compiling jars, so I was wondering if you were also planning to have a new jar release, or if you might be willing to share a jar with those changes incorporated? Or should we use snapshot jar that you sent before?

Thanks for all your time and help!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

PouletAxel commented 2 years ago

I made the new release. It should work it the same snapshot.jar I send you last time. I will close the issue but fill free to contact us if you need something else