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

feat: errors are emitted from asyncronous tasks #25

Merged
merged 4 commits into from Mar 30, 2022

Conversation

mmolari
Copy link
Collaborator

@mmolari mmolari commented Mar 14, 2022

In the align function (align.jl)

  • added @sync macro on the loop in which threads are spawned.
  • removed the try-catch (obsolete?)

mmolari and others added 2 commits March 14, 2022 20:43
In the `align` function (`align.jl`)
-  added `@sync` macro on the loop in which threads are spawned.
- removed the try-catch (obsolete?)
replace with just a local variable
@nnoll
Copy link
Collaborator

nnoll commented Mar 18, 2022

This looks good to me. I think try it on a few more cases and merge it. It's a rather essential fix so should take top priority.

this channel was used to return the final graph, and is now not needed
anymore.
alignment tasks that run in parallel have a try-catch statement.
If an error occurs, it is passed to an error channel, together with
the stacktrace. The main thread waits on this error channel,
and logs any error received and throws a new error.
The error channel is closed in the last alignment taks,
when the root graph is created.
@mmolari
Copy link
Collaborator Author

mmolari commented Mar 20, 2022

With this last commit now this looks good to me. If errors occur in any of the alignment tasks, they are passed on an error channel to the main thread. The main threads logs the error and stacktrace on stderr, and then throws a new error, stopping the execution.

@mmolari mmolari marked this pull request as ready for review March 20, 2022 11:55
@mmolari mmolari merged commit c0662df into master Mar 30, 2022
@mmolari mmolari deleted the feat/emit-thread-errors branch March 30, 2022 08:17
mmolari added a commit that referenced this pull request Oct 12, 2022
## Changes:

- added [mmseqs2](https://github.com/soedinglab/MMseqs2) as an alternative alignment kernel that guarantees higher sensitivity at the expense of longer computational time, see [#33](#33).
- updated Docker file to include mmseqs2 in the container.
- updated the documentation, including discussion of alignment kernel sensitivities and examples of application of PanGraph to plasmids by [@liampshaw](https://github.com/neherlab/pangraph/commits?author=liampshaw).
- errors that occur in worker threads are now emitted on the main thread, see [#25](#25).
- fixed a bug in detransitive, see this [commit](a965132)
- added snakemake pipeline in the `script` folder to perform the analysis published in our [paper](https://github.com/neherlab/pangraph#citing).
- added `-K` option to the `build` command to control kmer length for mmseqs aligner, see this [commit](0857c36).
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.

None yet

2 participants