Make ControlFREEC work for WES and WGS data in both paired and tumor-only modes #302

jfnavarro · 2020-11-07T15:00:13Z

nf-core/sarek pull request

Control-FREEC has been upgraded and the configuration of its process has been optimized and updated to work with WES data as well as WGS data. Few bugs have been fixed and improvements have been made in the code and docs. Tumor-only mode for Control-FREEC has been added too.

PR checklist

This comment contains a description of changes (with reason)
If you've fixed a bug or added code that should be tested, add tests!
If necessary, also make a PR on the nf-core/sarek branch on the nf-core/test-datasets repo
Ensure the test suite passes (nextflow run . -profile test,docker).
Make sure your code lints (nf-core lint .).
Documentation in docs is updated
CHANGELOG.md is updated
README.md is updated

Learn more about contributing: CONTRIBUTING.md

… the ControFREEC process

maxulysse · 2020-12-09T10:50:05Z

@jfnavarro with the help of @ewels we finally fixed #305 issue
So instead of removing the mappability option from Control-FREEC, would it be possible to make it as an optional parameter when the file is not provided?

jfnavarro · 2020-12-10T10:36:38Z

@jfnavarro with the help of @ewels we finally fixed #305 issue
So instead of removing the mappability option from Control-FREEC, would it be possible to make it as an optional parameter when the file is not provided?

Yes, of course.

…ES in Control-FREEC

maxulysse · 2020-12-10T12:44:01Z

conf/igenomes.config

@@ -63,7 +63,6 @@ params {
      intervals               = "${params.igenomes_base}/Mus_musculus/Ensembl/GRCm38/Annotation/intervals/GRCm38_calling_list.bed"
      known_indels            = "${params.igenomes_base}/Mus_musculus/Ensembl/GRCm38/MouseGenomeProject/mgp.v5.merged.indels.dbSNP142.normed.vcf.gz"
      known_indels_index      = "${params.igenomes_base}/Mus_musculus/Ensembl/GRCm38/MouseGenomeProject/mgp.v5.merged.indels.dbSNP142.normed.vcf.gz.tbi"
-      mappability             = "${params.igenomes_base}/Mus_musculus/Ensembl/GRCm38/Annotation/Control-FREEC/GRCm38_68_mm10.gem"


This line should not be deleted

Uppps :) Fixed!

maxulysse · 2020-12-10T12:46:29Z

main.nf

@@ -38,7 +38,7 @@ def helpMessage() {
      -profile                      [str] Configuration profile to use. Can use multiple (comma separated)
                                          Available: conda, docker, singularity, test, awsbatch, <institute> and more
      --step                       [list] Specify starting step (only one)
-                                          Available: mapping, prepare_recalibration, recalibrate, variant_calling, annotate, Control-FREEC
+                                          Available: mapping, prepare_recalibration, recalibrate, variant_calling, annotate, ControlFREEC


Suggested change

Available: mapping, prepare_recalibration, recalibrate, variant_calling, annotate, ControlFREEC

Available: mapping, prepare_recalibration, recalibrate, variant_calling, annotate, Control-FREEC

Actually as for tools, the string is stripped out of _ and - and lowercased, so Control-FREEC should actually work.
I would prefer that spelling as it's the one from the tool itself

maxulysse

Looks good to me.
@szilvajuhos any comments?

szilvajuhos

Looks fine, though we have changed the coefficientOfVariation for a reason I guess. It needs testing anyway.

szilvajuhos · 2020-12-10T13:50:47Z

nextflow.config

@@ -49,9 +49,9 @@ params {
  // Variant Calling
  ascat_ploidy = null // Use default value
  ascat_purity = null // Use default value
-  cf_coeff = "0.015"  // default value for Control-FREEC
+  cf_coeff = "0.05"  // default value for Control-FREEC


Dunno, what is the default value, and why exactly are we changing it?

We are changing it to the default value as recommend in their manual (http://boevalab.inf.ethz.ch/FREEC/tutorial.html). Was there any specific reason to use a value different than the default one?

Well, our colleagues played with that, and were happier with 0.05. Will ask why.

I see :) I personally believe that is nice to have as default the same as in Control-FREEC specially given that this parameter can be changed by the user. The optimal value is probably different for every case but of course I may be wrong here.

As I check the Control-FREEC source, this value is used only at one place: https://github.com/BoevaLab/FREEC/blob/master/src/GenomeCopyNumber.cpp#L60 and the sole purpose is to calculate the windowSize, if it is not provided in the config. For the clarity, it is actually genome_size/(read_number*cf_coeff^2) and my guess the readlength should be considered somewhere too, but whatever. Smaller windowSize should mean higher resolution but more noise. We can use the default, sure, and change the value in our own config. I can run some tests on our data, to be safe before release.

Interesting, the recommended setting for WES is to use cv_window = 0. I added an entry about that in the docs. It may be a good idea to mention this for the cv_coef parameter.

main.nf

maxulysse · 2020-12-11T08:50:03Z

@szilvajuhos @jfnavarro any need for more modifications? or are we good to go for a merge?

…d unstable

szilvajuhos · 2020-12-16T08:16:08Z

Hej, I just do not know, for T/N samples it is not working for me with 11.6. Whatever, can we change https://github.com/jfnavarro/sarek/blob/dev/main.nf#L3421 and https://github.com/jfnavarro/sarek/blob/dev/main.nf#L3454 to something like

LINEWIDTH=`head -1 ${cnvTumor}| wc -w`; awk 'NF=='$LINEWIDTH'{print}'  ${cnvTumor} > TUMOR.CNVs

so it should work for any fileformat hopefully.

…e. Relax cpu requirements in ControlFREEC

szilvajuhos

espléndido

jfnavarro added 4 commits November 7, 2020 15:35

Remove mappability option for ControlFREEC

3909d8f

Remove mappability option for ControlFREEC

90a9587

Remove mappability for ControlFREEC

d838905

Remove mappability option for ControlFREEC

864c7e8

jfnavarro requested a review from maxulysse as a code owner November 7, 2020 15:00

jfnavarro added 2 commits November 12, 2020 16:45

Make ControlFREEC compatible with WES

6911bf3

Better support for exome data in ControlFREEC

04fb29a

maxulysse mentioned this pull request Nov 17, 2020

Update environment.yml #304

Merged

8 tasks

jfnavarro added 11 commits November 17, 2020 13:33

Bump ControlFREEC version and fix a bug with the output file names of…

bc17ae9

… the ControFREEC process

Fix a couple of typos

5091752

Fixing minor bugs in the parameters for ControlFREEC

22b18c7

Update docs with new ControlFREEC configuration

fcaca32

Fixed a small typo

31aaefe

Remove unused setting in ControlFREEC

62fd810

More fixes to make ControlFREEC work

c1eef46

Fix a bug in the ControlFREECViz process taking wrong channel

47de309

Ensure the right channel in ControlFREECViz

b0a0c76

Fix a bug in the ControlFREECViz process

dd2d403

Fix a bug in ControlFReecViz

9d01fd3

Restored mappability for Control-FREEC. Changed subclone option for W…

acd0ea5

…ES in Control-FREEC

maxulysse reviewed Dec 10, 2020

View reviewed changes

Restoring igenomes mappability for the mouse reference

c8ae43d

maxulysse reviewed Dec 10, 2020

View reviewed changes

jfnavarro added 4 commits December 10, 2020 13:53

Update usage.md

4fac08c

Update nextflow_schema.json

dbd8537

Add ControlFREEC process for tumor-only samples

45f3502

Merge branch 'dev' of github.com:jfnavarro/sarek into dev

687caa9

szilvajuhos approved these changes Dec 10, 2020

View reviewed changes

jfnavarro changed the title ~~Remove mappability option from ControlFREEC~~ Make ControlFREEC work for WES and WGS data in both paired and tumor-only modes Dec 10, 2020

jfnavarro added 2 commits December 10, 2020 15:37

Change name of mappability variable in ControlFREEC

0774484

Fix an issue with the duplication of the channel in ControlFREECSingle

fdca828

maxulysse reviewed Dec 10, 2020

View reviewed changes

main.nf Outdated Show resolved Hide resolved

Fix an error with comments in ControlFREEC

0cc2d82

maxulysse approved these changes Dec 11, 2020

View reviewed changes

szilvajuhos approved these changes Dec 11, 2020

View reviewed changes

Disabling subclone computation in ControlFREEC as it is still beta an…

4e3202a

…d unstable

ControlFreecViz works regardless of the output format in the CNVs fil…

ce5cc36

…e. Relax cpu requirements in ControlFREEC

szilvajuhos approved these changes Dec 18, 2020

View reviewed changes

maxulysse merged commit e94d595 into nf-core:dev Dec 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make ControlFREEC work for WES and WGS data in both paired and tumor-only modes #302

Make ControlFREEC work for WES and WGS data in both paired and tumor-only modes #302

jfnavarro commented Nov 7, 2020 •

edited

Loading

maxulysse commented Dec 9, 2020

jfnavarro commented Dec 10, 2020

maxulysse Dec 10, 2020

jfnavarro Dec 10, 2020

maxulysse Dec 10, 2020

jfnavarro Dec 10, 2020

maxulysse left a comment

szilvajuhos left a comment

szilvajuhos Dec 10, 2020

jfnavarro Dec 10, 2020

szilvajuhos Dec 10, 2020

jfnavarro Dec 10, 2020

szilvajuhos Dec 11, 2020

jfnavarro Dec 11, 2020

maxulysse commented Dec 11, 2020

szilvajuhos commented Dec 16, 2020

szilvajuhos left a comment

	Available: mapping, prepare_recalibration, recalibrate, variant_calling, annotate, ControlFREEC
	Available: mapping, prepare_recalibration, recalibrate, variant_calling, annotate, Control-FREEC

Make ControlFREEC work for WES and WGS data in both paired and tumor-only modes #302

Make ControlFREEC work for WES and WGS data in both paired and tumor-only modes #302

Conversation

jfnavarro commented Nov 7, 2020 • edited Loading

nf-core/sarek pull request

PR checklist

maxulysse commented Dec 9, 2020

jfnavarro commented Dec 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxulysse left a comment

Choose a reason for hiding this comment

szilvajuhos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxulysse commented Dec 11, 2020

szilvajuhos commented Dec 16, 2020

szilvajuhos left a comment

Choose a reason for hiding this comment

jfnavarro commented Nov 7, 2020 •

edited

Loading