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

Added bump2version config and explanation #282

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mauvilsa
Copy link

@mauvilsa mauvilsa commented Jul 27, 2021

This pull request adds a bump2version config so that the increase of versions automatically creates a git tag and a commit with a standard message.

This relates to issues #281 and #237.

@mauvilsa mauvilsa force-pushed the release-tags-using-bumpversion branch from 46a5353 to 2b1c70b Compare July 27, 2021 07:49
@bmilde
Copy link
Contributor

bmilde commented Aug 9, 2021

Thanks for the suggestion! Would you have a good idea on how to solve versioning for the kaldi dependency? Basically each pykaldi version needs to be compiled to a specific pykaldi compatible kaldi version: https://github.com/pykaldi/kaldi

It can't be mixed with vanilla Kaldi, as there are quite a few pykaldi specific changes to get clif to work with it.

Also sometimes small fixes get added to the kaldi dependency as well and if doesn't break compatibilty we don't bumb the pykaldi versioning. This happened happened in the 0.2.1 release.

Right now you can only reliably build the latest version from git, which isn't ideal I guess. The build scripts just refence the latest git version (in install_kaldi.sh).

@mauvilsa
Copy link
Author

mauvilsa commented Aug 17, 2021

@bmilde

Would you have a good idea on how to solve versioning for the kaldi dependency? Basically each pykaldi version needs to be compiled to a specific pykaldi compatible kaldi version

If git tags are created for the releases of pykaldi, people can checkout the tag they need and run a build script that would use the respective kaldi version that it requires. Though I thought that you were planning to post packages in pypi. Look at what I commented in #261 (comment). The package could include the kaldi files it requires so people don't even need to worry about kaldi or other dependencies, they just do pip3 install pykaldi.

It can't be mixed with vanilla Kaldi, as there are quite a few pykaldi specific changes to get clif to work with it.

I don't see why there is not even an attempt at upstreaming into https://github.com/kaldi-asr/ the required changes for clif. Having a parallel version of kaldi just adds maintenance cost. And the same can be said about clif. Why use a modified version of clif instead of using some stable version from https://github.com/google/clif?

@tjysdsg
Copy link

tjysdsg commented Aug 26, 2021

I don't think we need a custom kaldi or clif anymore.

For kaldi, the modification was to fix some swapping calls. But they are already fixed in the lastest version of kaldi: https://github.com/kaldi-asr/kaldi/blob/master/src/cudamatrix/cu-packed-matrix.cc#L121 and https://github.com/kaldi-asr/kaldi/blob/master/src/cudamatrix/cu-vector.cc#L1046

For clif, the modification was to avoid using async keyword as an argument name, but google has already fixed that: https://github.com/google/clif/blob/main/clif/python/gen.py#L989

@mauvilsa
Copy link
Author

I don't think we need a custom kaldi or clif anymore.

If this is already the case, it is great! But it would be good to change pykaldi such that it starts using stable versions from those repos.

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

3 participants