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.

Sign up for GitHub

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

Jump to bottom

Diff bind reorg #49

Merged
merged 53 commits into from
Oct 28, 2024
Merged

Diff bind reorg #49

merged 53 commits into from
Oct 28, 2024

Conversation

rroutsong
Copy link
Collaborator

@rroutsong rroutsong commented Oct 24, 2024 •
edited
Loading

Reorganization of diffbind + uropa rule sets

Minor bugs and issues addressed along the way

rroutsong and others added 30 commits August 15, 2024 18:14
@rroutsong
feat: add cftoolkit v0.6.0 w rmarkdown
4e0bdc9
@rroutsong
fix: inherit from docker v0.5.0
73f27db
@rroutsong
feat: move to static images for diff bind and uropa rules
8f402ec
@rroutsong
fix: rework diff bind and uropa scripts for modularity and new rules
368a33a
@rroutsong
fix: fix hooks, split diff bind and uropa into multiple rule hierarch…
08ad2fa 
…ical rules
@rroutsong
chore: remove planned new docker version
c1c7ff2
@rroutsong
fix: full path for config
bbf2310
@rroutsong
fix: set cwd at rule instead of script
d641467
@rroutsong
fix: cd to output dir in diff bind rule
0f78a9e
@rroutsong
fix: make sif cache aware of env
b77b72e
@rroutsong
fix: review changes
746452f
@rroutsong
fix: syntax error with rmarkdown::render
8ce07d2
@rroutsong
fix: images not image
9d200d7
@rroutsong
fix: remove references to container sif files
c16ac7b
@rroutsong
fix: pathing in rmarkdown and syntax in prep_diffbind
cacbd3a
@rroutsong
fix: split diffbind rules into multiple use cases
2063dbd
@rroutsong
fix: retool rule resources
f2d3e83
@rroutsong
chore: merge main
b9df531
@rroutsong
fix: testing changes
74d241b
@rroutsong
fix: finalize validation fixes
5e847c5
@rroutsong
fix: additional uropa_prep_in updates
1398faa
@tovahmarkowitz
Update DiffBind_v2_EdgeR_block.Rmd
e0db6d0 
Removing up.bed and down.bed
@rroutsong
fix: take output from diffbind_count in to uropa
b87ef5a
@rroutsong
Merge branch 'diff_bind_reorg' of http://github.com/OpenOmics/chrom-seek
ea3f40d 
 into diff_bind_reorg
@rroutsong
fix: remove up down bed outputs from edger blocking rule
6844d88
@rroutsong
chore: begin to seperate uropa prep inputs
8a3f9ac
@rroutsong
fix: rereouting everything to single application rules
61901ec
@rroutsong
fix: wildcard correction for expanded dba rules
20e3d35
@rroutsong
chore: resource allocation for dba rules update
d0bdfd0
@rroutsong
feat: validation of cfchip pipeline
e4b6c2a
@rroutsong rroutsong self-assigned this Oct 24, 2024
tovahmarkowitz
tovahmarkowitz requested changes Oct 25, 2024
View reviewed changes
Copy link
Collaborator

@tovahmarkowitz tovahmarkowitz left a comment

Choose a reason for hiding this comment

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

Hi Ryan,

This looks great! Just two things I really want fixed for this pull request, which are associated with prep_diffbind.py and the cluster.json.

Tovah

bin/prep_diffbind.py Show resolved Hide resolved
config/cluster.json Show resolved Hide resolved
workflow/rules/qc.smk Show resolved Hide resolved
workflow/rules/dba.smk Show resolved Hide resolved
tovahmarkowitz
tovahmarkowitz requested changes Oct 25, 2024
View reviewed changes
config/cluster.json Show resolved Hide resolved
tovahmarkowitz
tovahmarkowitz approved these changes Oct 25, 2024
View reviewed changes
Copy link
Collaborator

@tovahmarkowitz tovahmarkowitz left a comment

Choose a reason for hiding this comment

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

Thank you!

skchronicles
skchronicles approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@skchronicles skchronicles left a comment

Choose a reason for hiding this comment

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

Everything looks good! The only suggestion I have which can be added later is to unset the R_LIBS_USER variable in the shell command of any rules relying on module load versions of R. If it's needed, we can add this with the next PR though.

src/run.py
# Using SINGULARITY_CONTAINALL or APPTAINER_CONTAINALL
# causes downstream using where $SLURM_JOBID is
# NOT exported within a container.
if 'R_LIBS_SITE' in my_env:
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we still relying on any module load R for any rules? If so, one thing that we should also do is unset R_LIBS_USER in any rule's shell commands that are using a module load verison of R.

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Copy link
Contributor

Choose a reason for hiding this comment

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

When R is module loaded, this environment variable is set, so even if you are unsetting this variable right before submission of the master job, if the rule module loads R, it will be reset. To handle this, we will need to unset that variable at the rule-level in the shell command right before we run anything R related.

workflow/rules/dba.smk Show resolved Hide resolved
@skchronicles skchronicles merged commit 8d9ffbb into main Oct 28, 2024
1 check passed
tovahmarkowitz added a commit to tovahmarkowitz/chrom-seek that referenced this pull request Oct 28, 2024
@tovahmarkowitz
Merge pull request #1 from OpenOmics/main
15c1e89 
Diff bind reorg (OpenOmics#49)
@tovahmarkowitz tovahmarkowitz mentioned this pull request Oct 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tovahmarkowitz tovahmarkowitz tovahmarkowitz approved these changes

@skchronicles skchronicles skchronicles approved these changes

Assignees

@rroutsong rroutsong

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rroutsong @skchronicles @tovahmarkowitz