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

Run miniprot only once #25

Merged
merged 10 commits into from
Mar 28, 2023
Merged

Run miniprot only once #25

merged 10 commits into from
Mar 28, 2023

Conversation

tomasbruna
Copy link
Contributor

Tested on test1.sh.

Also tested on large protein inputs - "miniprothint gtf" was always matching the one miniprot would produce natively.

@tomasbruna
Copy link
Contributor Author

tomasbruna commented Mar 26, 2023

Ahh, the previous implementation would not work if multiple separate protein files were on input. That's fixed.

The current implementation might still run into issues if the uptodate( [ $prot_seq_files[$i] ], [$prot_hintsfile] is triggered. I'll look into this more.

@tomasbruna
Copy link
Contributor Author

Also, I realized IDs might clash if miniprot is run separately on multiple files (that's related to the original implementation as well). tomasbruna/miniprothint@a38f300 should take care of that.

@KatharinaHoff
Copy link
Member

The uptodate stuff does not work in all places, anyway. I should go through the code and remove it... in the early days for BRAKER, when we had a lot of crashes, we had that idea that users would continue a failed run after fixing BRAKER... but as I said: it's buggy, either way.

The current implementation might still run into issues if the uptodate( [ $prot_seq_files[$i] ], [$prot_hintsfile] is triggered. I'll look into this more.

@KatharinaHoff
Copy link
Member

Our HPC is going into maintenance, tomorrow morning. I will run the tests on our end before merging. So the merge might take 2-3 days from now (depending on when our HPC awakens from its upgrade).

Thank you so much for all the work that you put into this, Tomas!!! This is really good.

@tomasbruna
Copy link
Contributor Author

The uptodate stuff does not work in all places, anyway. I should go through the code and remove it...

I was hoping you'd say that :), it seems like a big headache to maintain it.

@KatharinaHoff
Copy link
Member

If we at some point in time fully port to python, then snakemake (or Nextflow) would take care of this... timeline for this is very unsure on our end, though. I remove uptodate from the code for now.

I was hoping you'd say that :), it seems like a big headache to maintain it.

@KatharinaHoff KatharinaHoff merged commit 2c355ca into main Mar 28, 2023
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