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

[r] Improve error printing in call_peaks_macs #175

Merged
merged 6 commits into from
Jan 9, 2025
Merged

Conversation

bnprks
Copy link
Owner

@bnprks bnprks commented Dec 21, 2024

I noticed that call_peaks_macs wasn't printing out helpful error messages because they get trapped inside the mclapply call sometimes. (This came up because I accidentally broke macs in my local virtualenv, but when tests failed there weren't good error messages)

Previous output when MACS hit an error while running peak calling:

Warning message:
In parallel::mclapply(file_names, function(shell_file) { :
  2 function calls resulted in an error

New output when MACS hits an error while running peak calling:

2024-12-20 16:01:21  Error running MACS for cluster: a_group
MACS log file written to: /tmp/RtmpwYXpma/file30a8322a9d7527/macs-test/output/a_group/log.txt
2024-12-20 16:01:21  Error running MACS for cluster: b_group
MACS log file written to: /tmp/RtmpwYXpma/file30a8322a9d7527/macs-test/output/b_group/log.txt
Error in `call_peaks_macs()`:
! MACS calls encountered 2 failures
• See error logs listed above
Run `rlang::last_trace()` to see where the error occurred.

The problem here is that if we throw an error inside a mclapply call, then it just reports there was some error but the text never gets printed

Copy link
Collaborator

@immanuelazn immanuelazn left a comment

Choose a reason for hiding this comment

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

This is a good catch. I recall a previous conversation in which we were tyring to fix this, and the log_progress() conditional for if (.Platform$GUI == "RStudio") ended up resolving it. It was probably an oversight on the first pass with not checking whether stop messages were also affected by multithreaded mclapply. It might make sense to flag message() and stop this to the r-source devs as these issues have existed for more than 10 years now.

@bnprks bnprks merged commit d3d3d35 into main Jan 9, 2025
4 checks passed
@bnprks bnprks deleted the bp/mclappy-error-print branch January 9, 2025 01:12
immanuelazn pushed a commit to immanuelazn/BPCells that referenced this pull request Jan 18, 2025
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.

2 participants