CDCgov / phoenix

🔥🐦🔥PHoeNIx: A short-read pipeline for healthcare-associated and antimicrobial resistant pathogens
Apache License 2.0
52 stars 19 forks source link

BBDUK #98

Closed slsevilla closed 1 year ago

slsevilla commented 1 year ago

When running v1.1.0 on a Linux-based operating system, I ran into the following error:

-[cdcgov/phoenix] Pipeline completed with errors-
Error executing process > 'PHOENIX:PHOENIX_EXTERNAL:BBDUK (2022020725)'

Caused by:
  Process `PHOENIX:PHOENIX_EXTERNAL:BBDUK (2022020725)` terminated with an error exit status (1)

Command executed:

  maxmem=$(echo "-5 GB"| sed 's/ GB/g/g')
  bbduk.sh \
      -Xmx$maxmem \
      in1=2022020725.R1.fastq.gz in2=2022020725.R2.fastq.gz \
      out1=2022020725_cleaned_1.fastq.gz out2=2022020725_cleaned_2.fastq.gz \
      threads=4 \
      hdist=1 k=31 \
      ref=phiX.fasta \
      &> 2022020725.bbduk.log

  cat <<-END_VERSIONS > versions.yml
  "PHOENIX:PHOENIX_EXTERNAL:BBDUK":
      bbmap: $(bbversion.sh)
  END_VERSIONS

Command exit status:
  1

Command output:
  (empty)

Looking at the logs I found the source was an additional "-" that was being added to my maxmem:

$ cat *log

java -ea -Xmx-5g -Xms-5g -cp /opt/bbmap/current/ jgi.BBDuk -Xmx-5g in1=2022020725.R1.fastq.gz in2=2022020725.R2.fastq.gz out1=2022020725_cleaned_1.fastq.gz out2=2022020725_cleaned_2.fastq.gz threads=4 hdist=1 k=31 ref=phiX.fasta
Invalid maximum heap size: -Xmx-5g
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

To resolve this I added an extra sed to the modules/local/bbduk.nf file (line 26):

maxmem=\$(echo \"$maxmem GB\"| sed 's/ GB/g/g' | sed 's/-//g')

Not quite sure why my maxmem is leaving in this extra "-", but I figured I'd bring it to your attention in case others found this bug too.

jvhagey commented 1 year ago

Yikes, ok that is due to lack of adequate memory being requested for that job. Are you using a custom config file? The bbduk and bbmap_reformat require a minimum of 14GB and 11GB respectively. This is in the base.config already so if you aren't using a custom config something is causing it to get the wrong memory requirement passed...

jvhagey commented 1 year ago

Hi @slsevilla, just wanted to follow up and see if increasing memory resolved this? we are going to put out a patch for another bug and would like to include a fix for this if we need to.

slsevilla commented 1 year ago

Yep, I'm using a custom config file. Turns out the error also displays for contig_less500.nf so I've added the same line of code there as well. Interestingly both were able to go to completion, despite being below the minimum threshold that you've mentioned.

I'm also still having the issue with the CUSTOM_DUMPSOFTWAREVERSIONS (issue #79) but I've already got a workaround from that one that I've just added back in.

jvhagey commented 1 year ago

Yea, it comes up in contig_less500.nf as it also uses bbtools. The min are set because I wanted to give those bbtools jobs more memory without having the java heap memory grow too much. So this line limits the java hemp mem to 2GB. During testing on our HPC, those jobs were requiring that much memory on enough samples it was worth bumping them up. What line did you add to address the problem?

slsevilla commented 1 year ago

For both modules/local/contig_less500.nf and modules/local/bbduk.nf I replaced:

    maxmem=\$(echo \"$maxmem GB\"| sed 's/ GB/g/g')

with this line, to remove the "-" that was being added from my custom config:

    maxmem=\$(echo \"$maxmem GB\"| sed 's/ GB/g/g' | sed 's/-//g')
jvhagey commented 1 year ago

Gotcha, so it just ends up passing 5GB then. Yea, that works you will just need to change it every time you get a new version of phx then if you don't want to increase the mem request in your config. As the issue has a fix I will go ahead and close this then.