Skip to content

fix(qualimap) Fix for errors in qualimap when memory is set to null - #12441

Open
bigbigdumdum wants to merge 2 commits into
nf-core:masterfrom
bigbigdumdum:fix_break_on_null_mem
Open

fix(qualimap) Fix for errors in qualimap when memory is set to null#12441
bigbigdumdum wants to merge 2 commits into
nf-core:masterfrom
bigbigdumdum:fix_break_on_null_mem

Conversation

@bigbigdumdum

Copy link
Copy Markdown

Changed task.memory.mega, which breaks if memory is set to null (as required in some HPCs), to if else block used in other other parts of nextflow.

The line

def memory = (task.memory.mega*0.8).intValue() + 'M'

Got changed to

def avail_mem = 3072
    if (!task.memory) {
        log.info('[Qualimap RNAseq] Available memory not known - defaulting to 3GB. Specify process memory requirements to change this.')
    } else {
        avail_mem = (task.memory.mega * 0.8).intValue()
    }

and corresponding calls to memory variables ($memory) got changed to $avail_mem

This was done for the following files

  • modules/nf-core/qualimap/bamqc/main.nf
  • modules/nf-core/qualimap/bamqccram/main.nf
  • modules/nf-core/qualimap/rnaseq/main.nf

lint identified issues with other lines irrelevant to these changes. I didn't look more into this.

qualimap/rnaseq │ modules/nf-core/qualimap/rnaseq/main.nf │ main_nf_meta_key: Invalid 'meta' keys detected: meta.strandedness, meta.strandedness

│ qualimap/bamqc │ modules/nf-core/qualimap/bamqc/main.nf │ main_nf_ext_key: Invalid 'ext' keys detected: ext.suffix, ext.suffix │
│ qualimap/bamqc │ modules/nf-core/qualimap/bamqc/main.nf │ main_nf_meta_key: Invalid 'meta' keys detected: meta.strandedness, meta.strandedness

│ qualimap/bamqccram │ modules/nf-core/qualimap/bamqccram/main.nf │ main_nf_meta_key: Invalid 'meta' keys detected: meta.strandedness, meta.strandedness │
│ qualimap/bamqccram │ modules/nf-core/qualimap/bamqccram/main.nf │ main_nf_version_topic: Module does not emit software versions as topic

nf-test could not be installed along with nf-core in my system (Ubuntu 24.04.3 LTS) within the same conda environment. So testing was not done.

check boxes that I was not sure about were left unchecked

PR checklist

Closes #XXX

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • Broadcast software version numbers to topic: versions - See version_topics
  • Follow the naming conventions.
  • Add a resource label
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test <MODULE> --profile docker
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • nf-core subworkflows test <SUBWORKFLOW> --profile conda

… set, to if else block used in other other parts of nextflow.

@erikrikarddaniel erikrikarddaniel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Difficult to make a test for, I suppose? Otherwise, LGTM, even if I have no idea whether the 3 GiB default is reasonable.

@mashehu

mashehu commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Please join the github org via the slack channel so the CI runs for you.

@bigbigdumdum

Copy link
Copy Markdown
Author

Difficult to make a test for, I suppose? Otherwise, LGTM, even if I have no idea whether the 3 GiB default is reasonable.

I assumed 3GB is standard. The modification is based on the comment from @pinin4fjords on the issue I posted in nf-core/rnaseq nf-core/rnaseq#1887.

But I think a memory value parameter which can be set in config should be a better way to do this than hard coding the value in each process. But this would need changes uniform changes to all the modules where this block of code is used (which I gather is ~126).

@bigbigdumdum

bigbigdumdum commented Jul 29, 2026

Copy link
Copy Markdown
Author

Hello! i have run the tests. And many seem to have failed. The linting fails are from preexisting code in the files. I do not know what the rest of the test are checking. Can you please take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants