Skip to content
This repository has been archived by the owner on Oct 13, 2022. It is now read-only.

upgrade to spoa 4.0.6 #224

Closed
wants to merge 4 commits into from
Closed

upgrade to spoa 4.0.6 #224

wants to merge 4 commits into from

Conversation

mr-c
Copy link
Contributor

@mr-c mr-c commented Jan 11, 2021

No description provided.

@paoloczi
Copy link
Contributor

Thank you for doing this - I am certainly open to upgrading Shasta to this more current version of spoa but we need to be careful that nothing breaks. Do you expect that Shasta would see improvements in assembly performance or quality as a result of upgrading to this new release?

Before merging this change we would need to make sure that the code still builds and works on all supported platforms. This would require building and testing on Ubuntu 20.04, macOS 10.14 and 10.15, and ARM Ubuntu 20.04. In this repository, we use GitHub Actions to do automatic builds on all of these platforms (except ARM) at each push, but I don't see that in the fork that you are merging from.

In addition to testing the builds, we would also need to verify, at least on a test assembly, that assembly quality and performance are equivalent or better. In the recent past, @bagashe has been handling the process of upgrading to new spoa versions, but he is no longer working on Shasta and I am currently too busy to do this myself. I am especially wary because of the API changes that resulted in the proposed changes in the Shasta C++ code.

So in summary, I am interested in getting this done, but I would need a bit more help from you to take care of the above. If you could check the build on all platforms and can provide me the resulting executables, I could do some tests of assembly quality and performance.

@mr-c
Copy link
Contributor Author

mr-c commented Jan 11, 2021

Thank you @paoloczi for you reply.

This patch is from the Debian Med team, you can see the result of building with it for Debian's development distribution at https://buildd.debian.org/status/package.php?p=shasta (it will migrate to Debian's Testing distribution in about 5 days, and then it will be part of the next stable release of Debian "Bullseye" in Q2.

Do you expect that Shasta would see improvements in assembly performance or quality as a result of upgrading to this new release?

I don't know. I'm not a user, I'm a volunteer with Debian. As Spoa was updated to version 4.0.6 we are updating all packages that use libspoa.

In this repository, we use GitHub Actions to do automatic builds on all of these platforms (except ARM) at each push, but I don't see that in the fork that you are merging from.

Looks like it wasn't configured to run on PRs from forks, so I added c7d6f3c to fix that

@paoloczi
Copy link
Contributor

paoloczi commented Jan 11, 2021

Thanks @mr-c! I don't want to run GitHub Actions on PRs from forks - only on pushes. Running it on PRs from forks creates a security exposure that could be used by hackers to execute arbitrary code under the repository's workflow.

I noticed that, after enabling GitHub Actions in your fork, you are getting some failures. Clearly, those would have to be fixed.

Also, my understanding is that migrating to the new version of spoa is not likely to provide accuracy or performance improvements to Shasta. Given the need for some testing and other ongoing activities, my inclination is to postpone this upgrade.

@mr-c
Copy link
Contributor Author

mr-c commented Jan 11, 2021

FYI, here are some Spoa changelogs entries since Spoa 3.4.0

Spoa v4.0.5

  • Fixed MSA indel tails, dispatch compilaton and handling of longer input sequences.

Spoa v4.0.3

  • Fixed FASTA input bug

Spoa v4.0.0

  • Added Cereal support, alignment scores, and mode in which both strands are aligned to the graph and the better alignment is picked.

@paoloczi
Copy link
Contributor

Thanks @mr-c! I wish I had some time to do the testing that this upgrade requires, but I simply don't. Upgrading without testing risks breaking functionality, and I don't want to take that risk. It is wonderful that Debian is investing in Shasta, but at least for now it is not feasible for me to put time to help with this effort. If a knowledgeable Shasta user is listening and is willing to help test this upgrade, please let me know and I can provide some guidance.

@paoloczi
Copy link
Contributor

So far nobody volunteered to do the necessary testing work, so I will plan on doing it at some point when I can find some time. However I cannot promise when that will be. I will leave this pull request open in the meantime.

The following two changes would be needed before I can merge this:

  1. Fixing the build failures.
  2. Undoing the workflow changes in .github/workflows/Build.yml. As I mentioned in an earlier comment, I don't want to run the build workflow on pull requests because of the security exposure this would create.

@paoloczi
Copy link
Contributor

paoloczi commented Mar 4, 2021

I will close this. Upgrading to a newer spoa version will have to be done at some point, but will require some testing and nobody is available to do this work.

@paoloczi paoloczi closed this Mar 4, 2021
@mr-c
Copy link
Contributor Author

mr-c commented Mar 4, 2021

@paoloczi can you keep this open so that it is available for the next person? It was a bit of work..

@paoloczi
Copy link
Contributor

paoloczi commented Mar 4, 2021

Sure, I can reopen it, but it will require more work, in addition to some testing, because last time I checked it was failing the Github Actions build. And it could require more work in the future as the code diverges from the commit that you branched from. But sure, I can reopen it.

@paoloczi paoloczi reopened this Mar 4, 2021
@paoloczi
Copy link
Contributor

The Shasta build is now ported to Ubuntu 22.04, which provides the spoa library as part of package libspoa_dev. I was able to make the necessary API changes and get the code to compile, but I can't link the Shasta static executable because the libspoa_dev package only includes the shared library, libspoa.so, but not the corresponding static library. I did not find the static library in any of the other spoa packages available on Ubuntu 22.04.

Given this, and given that the static executable is the primary release artifact for Shasta, I will continue to have to build the spoa library from source (in shasta/scripts/InstallPrerequisites-Ubuntu.sh). However I will be able to upgrade to using a more recent spoa release, instead of spoa 3.4.0 which is currently used in Shasta builds.

The best solution would be to add the static library to the official package. But even if you were to make this change now in Debian I am not sure if Ubuntu will pick it up now or wait for next release.

@paoloczi
Copy link
Contributor

By the way, the code changes I made to get it to compile are essentially identical to yours. But given that this PR is now very old I will prefer to make the changes myself in the current code base, instead of trying to merge your PR.

But we need to decide what to do about the static library first.

@paoloczi
Copy link
Contributor

I made changes essentially equivalent to these in commit f3eb4da to upgrade Shasta to use the latest spoa version 4.0.8.

For the future, I would prefer to use the Debian package for spoa instead of having to build the spoa library from source (which happens in shasta/scripts/InstallPrerequisites-Ubuntu.sh). However, this is not currently possible because the Debian package does not include the static library.

Thank you @mr-c for pushing for this, and even better if you could convince the right person to add the static library to the spoa package.

@paoloczi paoloczi closed this Jul 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants