Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixing of function scope to two separate calls to same Groovy function from separate workflows #5088

Open
bskubi opened this issue Jun 24, 2024 · 2 comments

Comments

@bskubi
Copy link

bskubi commented Jun 24, 2024

Bug report

This is a very peculiar bug involving an apparent breakdown of scope when the same Groovy method is called by two subworkflows in parallel. The smallest example I've been able to produce is unfortunately still 291 lines long, but I have isolated the problematic lines and have an example that reproducibly demonstrates the error.

I've done my best to explain the problem clearly, but I understand that 291 lines of code is a lot for a 'minimal' reproducible example, so I'm happy to answer clarifying questions.

Expected behavior and actual behavior

I have a Groovy method, sqljoin, called from two separate places:

  1. From a subworkflow, RefsToFile
  2. From a second Groovy method, MakeResourceFile, which is in turn called from a second subworkflow MakeMissingChromsizes

Hence, we have:
RefsToFile -> sqljoin
MakeMissingChromsizes -> MakeResourceFile -> sqljoin

For illustrating this example, there are two key arguments to sqljoin:

  1. caller is a string passed from the calling function ("RefsToFile" and "MakeResourceFile")
  2. kwargs is a hashmap from which is extracted the variable by.
  • In the case of "RefsToFile", the intended behavior is: by = "sample_id"
  • In the case of "MakeResourceFile", the intended behavior is: by = [assembly]

The expected behavior of the call to sqljoin from "RefsToFile" is observed if the call from "MakeResourceFile" is commented out. It is also correct up through line 99 of the example code. However, on line 100 and 102, two problems arise when the call from "MakeResourceFile" is not commented out (so that sqljoin is called from both places).

  1. The value stored in the caller variable is changed to equal "RefsToFile" in the call to sqljoin initiated by MakeResourceFile
  2. The value stored in the by variable is changed to equal [assembly] in the call to sqljoin initiated by RefsToFile.

This appears to represent a breakdown of scope between the two calls to sqljoin.

This is the first time I've managed to isolate this issue with 100% reproducibility and pinpoint the line at which the problem occurs, but I have observed similar errors in the past that all involved calling a custom Groovy method repeatedly in parallel. For example, in the past, I had a custom Groovy method that was called from within the shell section of a process where calls would appear to have the same variable value-swapping problem. The problem was eliminated by moving the code contained within the Groovy method directly into the shell section.

Steps to reproduce the problem

Extract and run the following code with nextflow run hich.nf
error.zip

This will illustrate the erroneous output. Then comment out the call to MakeResourceFile and rerun to show how this yields different results from the preceding call to sqljoin from the RefsToFile subworkflow.

Program output

(skubi) benjamin@Odysseus:~/Documents/error$ nextflow run hich.nf

Erroneous result illustrating mixup of variable values when call to sqljoin from MakeResourceFile is active

 N E X T F L O W   ~  version 24.04.2

Launching `hich.nf` [determined_spence] DSL2 - revision: e6e5da6a8d

RefsToFile by = sample_id
[-        ] MakeMissingChromsizes:MakeChromsizes -
MakeResourceFile by = [assembly]
right caller by=[assembly] after RefsToFile [[:], [sample_id:sample1]]
right caller by=[assembly] before RefsToFile [sample_id:sample1]
right caller by=[assembly] after RefsToFile [[:], [sample_id:sample2]]
right caller by=[assembly] before RefsToFile [sample_id:sample2]

Expected result when call to sqljoin from MakeResourceFile is commented out.

RefsToFile by = sample_id
right caller by=sample_id before RefsToFile [sample_id:sample1]
right caller by=sample_id after RefsToFile [[sample_id:sample1], [sample_id:sample1]]
right caller by=sample_id before RefsToFile [sample_id:sample2]
right caller by=sample_id after RefsToFile [[sample_id:sample2], [sample_id:sample2]]
[[[sample_id:sample1], [[sample_id:sample1, datatype:fastq, assembly:hg38, reference:hg38.fna, chromsizes:, index_dir:resources/index/bwa-mem2, index_prefix:hg38_noalts, data1:fastq/KO1.fq.gz, data2:fastq/KO2.fq.gz, enzymes:Arima, fragfile:, condition:1, biorep:1]]], [[sample_id:sample1], [sample_id:sample1]]]
[[[sample_id:sample2], [[sample_id:sample2, datatype:fastq, assembly:hg38, reference:hg38.fna, chromsizes:, index_dir:resources/index/bwa-mem2, index_prefix:hg38_noalts, data1:fastq/KO1.fq.gz, data2:fastq/KO2.fq.gz, enzymes:Arima, fragfile:, condition:1, biorep:1]]], [[sample_id:sample2], [sample_id:sample2]]]

Environment

  • Nextflow version: 24.04.2
  • Java version: openjdk version "20.0.2-internal" 2023-07-18
  • Operating system: Ubuntu Linux
  • Bash version: GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)

Additional context

The aim of the sqljoin method is to implement left, right and inner joins on channels that contain a single LinkedHashMap item. I am aware that Nextflow provides operators providing similar features, but this function uses them to implement additional new functionality not provided by Nextflow's basic operators.

@bentsherman
Copy link
Member

See the warning at the end of this section: https://nextflow.io/docs/latest/script.html#closures

Unfortunately Nextflow will allow you to declare variables without def but in the case of functions it will be a global (i.e. script-scope) variable. This doesn't happen for processes and workflows because they are scoped to the process/workflow instead.

I'm not certain but I have a feeling you are declaring a variable in your function without def and it is causing a race condition

@bskubi
Copy link
Author

bskubi commented Jun 25, 2024

That's correct, I wasn't using def inside of the function. That explanation makes sense. Thank you!

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

No branches or pull requests

2 participants