https://nf-co.re logo
Join Slack
Powered by
hi Duckies.....talking out loud so it will help me...
# rubber-duck
s
hi Duckies.....talking out loud so it will help me process: I am having issue with #CP3RJSMF0 
v2.7.1
where the bismark align step is not getting cached and is being re-run upon resume. I ran the 
test,debug
profile, and noticed that the hashes for the bismark Index folder input seems to vary, which I believe is invalidating the cache and causing a re-run. Q is - why is the bismark index folder getting a diff hash each time even with the path to the stage folder being the same ? 
run-1
hash
Copy code
e0e6778c9d458895209ee2555180d687 [nextflow.util.ArrayBag] [FileHolder(sourceObj:/workspace/methylseq/work/48/07893206ffcec48f5d0061c928db23/BismarkIndex, storePath:/workspace/methylseq/work/48/07893206ffcec48f5d0061c928db23/BismarkIndex, stageName:BismarkIndex)]
resume-1
hash
Copy code
2d2d4fcf2bffcae17ace858866c308ee [nextflow.util.ArrayBag] [FileHolder(sourceObj:/workspace/methylseq/work/48/07893206ffcec48f5d0061c928db23/BismarkIndex, storePath:/workspace/methylseq/work/48/07893206ffcec48f5d0061c928db23/BismarkIndex, stageName:BismarkIndex)]
resume-2
hash
Copy code
b6b7a8001ea430230fd4d49bddab8521 [nextflow.util.ArrayBag] [FileHolder(sourceObj:/workspace/methylseq/work/48/07893206ffcec48f5d0061c928db23/BismarkIndex, storePath:/workspace/methylseq/work/48/07893206ffcec48f5d0061c928db23/BismarkIndex, stageName:BismarkIndex)]
anybody have ideas on where should I be looking to pinpoint ? one change we introduced in 
2.7.1
was this force soft-linking of fasta file into the index folder to avoid the issues with nextflow nested-symlinks
Copy code
ln -sf \$(readlink $fasta) $index/$fasta
but, still that doesn't explain why the 
BismarkIndex
is getting a diff hash each time 🤷
l
I had some trouble with my fasta channel some times ago quite similar. The issue was fixed by adding 
cache = lenient
on the config file of my processes. Does this help ?
s
im seeing from comments > The task hash includes the lastModified of the file could the force link 
ln -sf
be modifying the task hash then ? checking...
hmmm.... 
cache = 'lenient'
seems to be working... good point @Louis Le Nézet.....thank you!
also multiqc in #CP3RJSMF0 also seems to have the same problem....not resuming @Phil Ewels
Copy code
executor >  local (2)
[06/bfa9b9] NFCORE_METHYLSEQ:PREPARE_GENOME:BWAMETH_INDEX (BwamethIndex/genome.fa)              [100%] 1 of 1, cached: 1 ✔
[-        ] NFCORE_METHYLSEQ:METHYLSEQ:CAT_FASTQ                                                -
[8c/12b486] NFCORE_METHYLSEQ:METHYLSEQ:FASTQC (Ecoli_10K_methylated)                            [100%] 1 of 1, cached: 1 ✔
[96/8668f5] NFCORE_METHYLSEQ:METHYLSEQ:BWAMETH:BWAMETH_ALIGN (Ecoli_10K_methylated)             [100%] 1 of 1, cached: 1 ✔
[d5/24c043] NFCORE_METHYLSEQ:METHYLSEQ:BWAMETH:SAMTOOLS_SORT (Ecoli_10K_methylated)             [100%] 1 of 1, cached: 1 ✔
[39/efcafa] NFCORE_METHYLSEQ:METHYLSEQ:BWAMETH:SAMTOOLS_INDEX_ALIGNMENTS (Ecoli_10K_methylated) [100%] 1 of 1, cached: 1 ✔
[0f/cbd892] NFCORE_METHYLSEQ:METHYLSEQ:BWAMETH:SAMTOOLS_FLAGSTAT (Ecoli_10K_methylated)         [100%] 1 of 1, cached: 1 ✔
[e5/1b6fa4] NFCORE_METHYLSEQ:METHYLSEQ:BWAMETH:SAMTOOLS_STATS (Ecoli_10K_methylated)            [100%] 1 of 1, cached: 1 ✔
[ab/c64095] NFCORE_METHYLSEQ:METHYLSEQ:BWAMETH:METHYLDACKEL_EXTRACT (Ecoli_10K_methylated)      [100%] 1 of 1, cached: 1 ✔
[5b/854227] NFCORE_METHYLSEQ:METHYLSEQ:BWAMETH:METHYLDACKEL_MBIAS (Ecoli_10K_methylated)        [100%] 1 of 1, cached: 1 ✔
[15/f8a48b] NFCORE_METHYLSEQ:METHYLSEQ:QUALIMAP_BAMQC (Ecoli_10K_methylated)                    [100%] 1 of 1, cached: 1 ✔
[7e/528289] NFCORE_METHYLSEQ:METHYLSEQ:PRESEQ_LCEXTRAP (Ecoli_10K_methylated)                   [100%] 1 of 1, failed: 1 ✔
[e5/5ee0f5] NFCORE_METHYLSEQ:METHYLSEQ:MULTIQC                                                  [  0%] 0 of 1
s
I thought multiqc was not supposed to cache
✅ 2
s
and why is that ?
now Q regarding bismark align - is 
cache = 'lenient'
an acceptable solution or should I dig deeper to find out exactly what is modifying the hash of the index folder ?
m
The 
ArrayBag
(nextflow version) always has a different hash because it doesn't hash like a normal Groovy collection. You need to make it a Groovy collection so it hashes properly for resume.
s
I see, is this ArrayBag a new change in nextflow ?
m
Nope
I bring this up as something to check nearly everytime someone talks about -resume not working
s
I asked as @Phil Ewels said this was not something that was seen previously
how do I make it not an ArrayBag ?
m
point me to the code line.
s
bismark/genomepreparation is creating the index and passing it to bismark/align
m
I'm trying to find which subworkflow it's in and I'm not finding it on github.dev
s
its 2 sub-wfs: prepare_genome and bismark
m
Why am I not finding it with find in files though? I type in BISMARK_GENOMEPREPARATION and reduce to files called 
*.nf
. It only finds the process declaration then on 
master
s
subworkflows/local/prepare_genome/main.nf
m
Hmm. There's no 
subworkflows/local
folder on 
<http://github.dev|github.dev>
s
m
or 
modules/local
for that matter either
s
there are no local modules in #CP3RJSMF0
m
Thanks for the link. I'm just wondering about 
<http://github.dev|github.dev>
I can't see where the 
ArrayBag
object is coming from.
thinker 1
Is the index in your test coming from 
BISMARK_GENOMEPREPARATION
or are you supplying a value to 
params.bismark_index
?
s
from 
BISMARK_GENOMEPREPARATION
even if it is supplied as param
m
Copy code
if (bismark_index) {
            if (bismark_index.endsWith('.gz')) {
                ch_bismark_index = UNTAR ([ [:], file(bismark_index) ]).untar
                ch_bismark_index.dump(tag: 'PREPARE_GENOME/UNTAR: ch_bismark_index')

                ch_versions      = ch_versions.mix(UNTAR.out.versions)
            } else {
                ch_bismark_index = Channel.value([[:], file(bismark_index, checkIfExists: true)])
                ch_bismark_index.dump(tag: 'PREPARE_GENOME: ch_bismark_index')
            }
        } else {
            BISMARK_GENOMEPREPARATION(ch_fasta)
Is what I see
s
yup
m
so if 
params.bismark_index
has a value then 
BISMARK_GENOMEPREPARATION
shouldn't run
s
no, it is still passed through the 
BISMARK_GENOMEPREPARATION
and this is set and emitted to next steps
Copy code
ch_bismark_index = Channel.value([[:], file(bismark_index, checkIfExists: true)])
☝️ prepare_genome is run irrespective
m
Yes, that I see, but not 
BISMARK_GENOMEPREPARATION
. That should only be run if 
params.bismark_index
is null
s
ah yes true
so this is test profile and 
BISMARK_GENOMEPREPARATION
is run to create index
m
And in this case 
BISMARK_GENOMEPREPARATION.out.index
is an implicit value channel because the input is a value channel which is why there's no 
combine
with the reads in the 
BISMARK
subworkflow
s
there is no combine of index with reads in bismark subworkflow (each is handled separately)...if ur seeing it...could you share the line ?
m
That's what I said. There's no 
combine
✅ 1
I would expect one normally
I'm wondering if it's because you pass the reference as a value channel instead of using something like 
fromPath
and then using 
collect
on the index
Maybe using 
collect
is might do it even if it is a value channel
s
lemme try
fromPath and collect doesn't do the trick
m
Yeah, I've been testing myself with a toy, and I can't get the ArrayBag hash to change
OK. I give up. I can't replicate it with a toy, and I can't figure it out from using the test profile either. I have no idea why that hashCode changes ( I tried investigating the List, checking if it's related to empty maps, and mapping to a new tuple, but nothing) I made an issue for it on nextflow issues: https://github.com/nextflow-io/nextflow/issues/5527
👍 1
s
Thank you @Mahesh Binzer-Panchal I ll dig into this again 2moro thinker
m
s
yes, I mentioned this initially above. I had my doubts about this too. We went down with this linking to avoid nested symlinks issue with nextflow
m
Oh, sorry, I had to arrive there myself ... Oh well. Yes, writing to a directory from another process output breaks reentrancy. Inputs should always be read-only. If you need a certain folder structure, you should remake it, and potentially delete it when the script is complete to save space if necessary.
👍 1
s
If you need a certain folder structure, you should remake it
how? mv/cp is not encouraged from my previous attempts
m
What exactly did you do?
I did:
Copy code
mkdir index
cp -r $index/* index
cp $fasta index

biskmark \
   --genome index

rm index
I think
s
ya thats for index creation, when you come to align having the fasta in the index folder has nested symlink issue whereby providing fasta via cli becomes an issue
that was the whole reason I went down this rabbithole rabbit hole
and bismark align needs fasta in the index folder or it wont work
but i see what you are suggesting is copy the fasta into the index folder...which also I proposed earlier but, I think there was a push to not duplicate inputs
m
If it's needed for the inputs, you don't have much choice.
s
What is the nested symlink issue?
Does it not manage when one symlink points to another symlink?
s
s
Oh, so the index folder is staged as a symlinked folder, but it wants the fasta file inside the same folder. What if you symlink in the folder, symlink in the fasta (separately), then make a new folder that has symlinks to those symlinks? Is that what fails?
s
yes that what we tried here...it works but then break resume
s
Copy code
mkdir index2
ln -s index index2/
ln -s fasta index2/
s
oh wait....don't think I tried 
make a new folder that has symlinks to those symlinks?
but still I think it will generate a new has each time
as just 
ln -s
was working in all cases before and had to 
ln- sf
to make it work. see that thread here
@Mahesh Binzer-Panchal I am thinking 
cp
the fasta or make align cache lenient ? which one would you recommend ?
m
cp it and remove it at the end.
✅ 1
s
cc @Phil Ewels @Maxime Garcia @Edmund we think that copying the fasta file into the index folder would be better until we have nextflow handle nested symlinks properly. I will make the change to the bismark align module if no objections
m
@Adam Talbot what do you think?
a
@Adam Talbot what do you think?
pigs are much bigger than you expect.
😂 1
(that's a very niche joke)
lemme read the thread
e
lemme read the thread
And he was never seen again
🤣 3
My thoughts(pardon if I'm missing anything you've tried): 1. 
stageAs
doesn't work, right? 2. We just build the index correctly in #C073ZDLNC92
3. If only we could somehow contact the Bismark author... and get a 
--fasta
flag added...
🙏 1
4. We scrap the bismark perl code, and rewrite it in Nextflow
🤯 1
Copy code
#!/usr/bin/env nextflow

nextflow.enable.dsl = 2

// Parameters
params.genome_folder = null
params.verbose = false
params.path_to_aligner = null
params.parallel = 1
params.single_fasta = false
params.bowtie2 = true
params.hisat2 = false
params.mm2 = false
params.slam = false
params.large_index = false
params.genomic_composition = false
params.outdir = 'results'

// Import modules
include { PREPARE_GENOME_FOLDERS } from './modules/prepare_genome_folders'
include { BISULFITE_CONVERT_GENOME } from './modules/bisulfite_convert_genome'
include { INDEX_GENOME } from './modules/index_genome'
include { CALCULATE_GENOMIC_FREQUENCIES } from './modules/calculate_genomic_frequencies'

// Main workflow
workflow {
    // Validate input
    if (!params.genome_folder) {
        error "Please provide a genome folder with --genome_folder"
    }

    // Step 1: Prepare genome folders
    PREPARE_GENOME_FOLDERS(params.genome_folder)

    // Step 2: Bisulfite convert genome
    BISULFITE_CONVERT_GENOME(PREPARE_GENOME_FOLDERS.out.genome_files)

    // Step 3: Index genome
    INDEX_GENOME(BISULFITE_CONVERT_GENOME.out.ct_converted, BISULFITE_CONVERT_GENOME.out.ga_converted)

    // Optional: Calculate genomic frequencies
    if (params.genomic_composition) {
        CALCULATE_GENOMIC_FREQUENCIES(params.genome_folder)
    }
}

// Print help message
def printHelp() {
    <http://log.info|log.info>"""
    Bismark Genome Preparation - Nextflow Version
    =============================================
    Usage:
    nextflow run <http://bismark_genome_preparation.nf|bismark_genome_preparation.nf> --genome_folder /path/to/genome

    Mandatory arguments:
      --genome_folder    Path to the folder containing the genome to be bisulfite converted

    Optional arguments:
      --verbose          Print verbose output for more details or debugging
      --path_to_aligner  The full path to the Bowtie 2, HISAT2 or minimap2 installation folder
      --parallel         Use several threads for each indexing process (default: 1)
      --single_fasta     Write converted genomes into single-entry FastA files
      --bowtie2          Create bisulfite indexes for Bowtie 2 (default: true)
      --hisat2           Create bisulfite indexes for HISAT2
      --mm2              Create bisulfite indexes for minimap2
      --slam             Transform T to C (forward strand) or A to G (reverse strand)
      --large_index      Force generated index to be 'large'
      --genomic_composition Calculate and extract the genomic sequence composition
    """
}

// Print help message if --help is used
if (<http://params.help|params.help>) {
    printHelp()
    exit 0
}

// Print version information if --version is used
if (params.version) {
    println "Bismark Genome Preparation - Nextflow Version ${workflow.manifest.version}"
    exit 0
}
s
pinging @Felix Krueger for the above idea of rewriting bismark in nextflow but, I think it will take time so, not ideal for now as I would like to get a patch release out asap where bismark can be resumed
I was wondering what are the downsides of enabling 
cache = lenient
for bismark align process ? (that way there will be no code change and can easily be removed in future)
this 3
p
Fine by me 👍🏻
✅ 1
I don't really understand what the "rewrite in Nextflow" thing means. Bismark does a lot of stuff that would be difficult if not impossible to do in Nextflow.
m
Then Rust
this 4
😂 2
a
I’m still lost. What’s the problem?
s
one change we introduced in #CP3RJSMF0 
2.7.1
was this force soft-linking of fasta file into the index folder to avoid the issues with nextflow nested-symlinks
Copy code
ln -sf \$(readlink $fasta) $index/$fasta
then, we found out that the force linking above is resulting a diff hash for the index folder each time in the align process thereby breaking nextflow resume
🤨 1
workarounds so far: • cp fasta into index folder and rm • lenient cache
f
wow, this was quite the read. I wonder if it would be easy to add an option 
--fasta
to Bismark what would allow one to specify the path to the FastA file (or files) separately, but default to the same folder as the index if not specified explicitly? Shouldn’t that ensure backwards compatibility while making staging easier within a Nextflow setting?
❤️ 1
m
Why is the 
ch_fasta
needed in 
BISMARK_ALIGN
? Isn't it already part of the 
BISMARK_GENOMEPREPARATION
?
f
Not sure if I am getting it right here, but Bismark expects the FastA files on the top level of the index folder, which makes it necessary to link or copy the .fa file into the index folder:
Copy code
.
├── Bisulfite_Genome
│   ├── CT_conversion
│   │   ├── BS_CT.1.bt2
│   │   ├── BS_CT.2.bt2
│   │   ├── BS_CT.3.bt2
│   │   ├── BS_CT.4.bt2
│   │   ├── BS_CT.rev.1.bt2
│   │   ├── BS_CT.rev.2.bt2
│   │   └── genome_mfa.CT_conversion.fa
│   └── GA_conversion
│       ├── BS_GA.1.bt2
│       ├── BS_GA.2.bt2
│       ├── BS_GA.3.bt2
│       ├── BS_GA.4.bt2
│       ├── BS_GA.rev.1.bt2
│       ├── BS_GA.rev.2.bt2
│       └── genome_mfa.GA_conversion.fa
└── E_coli_K12.fa
If the .fa file could come from anywhere then it would circumvent the requirement
this 1
m
Can't the 
BISMARK_GENOMEPREPARATION
already place it there since it's input is the fasta?
f
If it gets run, I assume it could. (and if you specify a certain path to the index to avoid having to re-run the indexing, one could make sure it is there, couldn’t one?) -> back over to @Sateesh_Peri :)
homer disappear 1
m
It was a whole mess
m
Seems to work, and yes, you could validate the Index if supplied has a certain format / contents
s
@Mahesh Binzer-Panchal what you proposed above works if the fasta file is being staged from remote and fails when providing a local path for the fasta file. can you try
Copy code
wget <https://github.com/nf-core/test-datasets/raw/methylseq/reference/genome.fa>
and run with 
--fasta genome.fa
and see if it works
m
Oh, interesting bug. The 
genome.fa
is there and readable but Bismark doesn't see it?
s
yes its the issue with nested symlinks...the reason all this started
m
Ah, now your comment above makes more sense to me
m
yeah, this whole thing was and still is a mess
I'm not happy with the solution we found
s
lets 
cp
the fasta into the index folder ? (the lenient cache was also not working all the time when i tested last friday)
m
So do you understand what's actually causing it the genome.fa not to be found?
m
not happy with that solution either
stageAs would be perfect, but I couldn't get it to work without colliding
s
m
OK. So it's a container thing. This works fine in conda
s
hmmm so if input is staged as 
path(fasta, stageAs: 'tmp/*')
, how is it linked into the index folder ?
m
It's not. It's just there so the container mounts the correct dir for original symlink to work
That's why the fasta is not `rm`'ed
s
oh i see....lemme try
m
Ooooh
That works on conda too?
m
Conda was never an issue, because the environment is not isolated
s
that seems to be working @Mahesh Binzer-Panchal....lemme do it again and confirm
👍🏽 1
need to fix bwameth next...will do that in a bit
m
Question is, is it a suitable solution to all instead of the 
cp
or whatever else was discussed here?
s
I would say its the best so far until nextflow static types are rolled out.... wat say @Maxime Garcia?
m
I think that's our best solution so far
s
Ya I'm going with that for bismark now. I am trying to fix it for bwameth subworkflow as well and noticed its not working the same as it did for bismark. Then, I realized bwameth align process does this touch of bwameth index files so, that bwameth doesn't complain about the index files being older than the fasta -> since the touch modifies the time stamp -> bwameth align process cache is modified and re-run each time This would mean that bwameth align didn't support resume since that touch command was added. Would I be right about this @Phil Ewels?
m
oh, that would make sense indeed
p
..because the inputs have changed for subsequent (re)runs?
the bwameth modification time check thing is so irritating
shakefist 1
party this 1
Might be nice to fix upstream by adding a config option to skip this check..
s
config option ? wouldn't it have to be commented out in the script ? then maintaining a separate container ?
p
I mean get Brent P to release a new version of bwa-meth where that check can be skipped
s
ah ya. that looks like the best path honestly
is Brent in our slack ?
s
config option ? wouldn't it have to be commented out in the script ? then maintaining a separate container ?
I mean, you can do things like: copy the python file out of the container into the working directory, edit the copy and then run it
🤔 1
wouldn't be particularly horrific
s
image.png
😂 2
e
You could probably just do a patch file with wave at that point, or edit the python file and build a container with it.
s
slightly bad
p
You could probably just do a patch file with wave at that point, or edit the python file and build a container with it.
Sux for non-container users tho
s
or non-wave users
p
or non-wave users
Nope as we're using Wave to build regular containers
Though actually it's using Seqera Containers, which we can't patch like this, so it'd have to be quay.io 👎🏻
sad cowboy 1
e
Sux for non-container users tho
You know what they say about that? Don't do that.
p
Better to fix upstream imho
s
I want seqera patchable containers! nerdy cowboy
p
That's called Wave 😅
😂 2
s
I have seen on other github issues it is an issue......now how do we get in touch with Brent P ?
p
He has been here on Slack in the past. Maybe start a clean thread though.
s
@Brent Pedersen
🕯️ 1
🙏 1
👍🏻 1
s
image.png
🤣 1
p
Might also be better off just opening a PR on the repo vs. asking him to do it on Slack
💯 1
eg. open a PR to fix, then try tagging him on Slack if no response for a while
ah, too late 😅
s
ya I look into opening a PR too 2moro
👍🏻 1
🚀 1
Open in Slack
PreviousNext
Powered by