search

kaldi-asr / kaldi

kaldi-asr/kaldi is the official location of the Kaldi project.
http://kaldi-asr.org
Other
14.11k stars 5.31k forks source link

Change log / Release #1943

Open vince62s opened 6 years ago

vince62s commented 6 years ago

This is just for discussion but feel free to close it of not relevant.

I am reacting to this new issue about backstich "best practice" as well as 2 new recent features that were commited: attention modelling layer #1731 and non splicing layers #1937 These are majors features IMO that are put in the middle of hundreds of commmits on this page: http://kaldi-asr.org/doc/versions.html where commits "fix typo" are at the same level as major features.

My suggestion, would be to use the "release" tab of github to at least list all major new features for each version. This is not a big maintenance, and would enable users to see at first sight what is really important when the repo is moving so fast....(for the good).

Cheers.

danpovey commented 6 years ago

It's not clear to me that the releases feature of github is really compatible with our workflow and other practices. But I'll try to think about the best way to give more visibility to important changes. Any concrete suggestions would be welcomed.

On Tue, Oct 17, 2017 at 3:22 PM, vince62s notifications@github.com wrote:

This is just for discussion but feel free to close it of not relevant.

I am reacting to this new issue about backstich "best practice" as well as 2 new recent features that were commited: attention modelling layer #1731 https://github.com/kaldi-asr/kaldi/pull/1731 and non splicing layers

1937 https://github.com/kaldi-asr/kaldi/pull/1937

These are majors features IMO that are put in the middle of hundreds of commmits on this page: http://kaldi-asr.org/doc/versions.html where commits "fix typo" are at the same level as major features.

My suggestion, would be to use the "release" tab of github to at least list all major new features for each version. This is not a big maintenance, and would enable users to see at first sight what is really important when the repo is moving so fast....(for the good).

Cheers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVuxLm64FMSHb5M4HCTPgLoPaDk1zoks5stP6GgaJpZM4P8sCe .

vince62s commented 6 years ago

My understanding is that this is just tags. You decide which commit is a version (major or minor but in Kaldi's case for sure not all minors) even starting with 5.1 and 5.2 and main major features would be great. Here is an example: https://github.com/tensorflow/tensor2tensor/releases But, yes this is manual.

kkm000 commented 5 years ago

We actually now have tags for 5.1 through 5.4. But 5.5 was not tagged. I'll take care of this, thanks. Since there appears to me a mash of commits after the 5.5 branch has been merged, I'll track this in a new issue.

danpovey commented 5 years ago

The tags represent the end of development of the respective branches. Because 5.5 is the current version, it corresponds to master, we'd only create the tag once we go to 5.6.

On Mon, Apr 1, 2019 at 7:05 AM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

Closed #1943 https://github.com/kaldi-asr/kaldi/issues/1943.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#event-2242943327, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVuzqERxOE-1UtKQ7wkoQODIBAN7mwks5vcefrgaJpZM4P8sCe .

kkm000 commented 5 years ago

This is a bit more complex with 5.5. A stable 5.5 is essentially the end of the branch plus 2 bug fixes that came in the following ~2 weeks. One is a memory leak, another broken compile on Windows. I'll explain in a ticket. I need to find them first.

danpovey commented 5 years ago

Are you sure you don't mean 5.4? I use the tags to mean the end of the development line for that branch, not the beginning. So there is rightfully no 5.5 tag currently.

On Mon, Apr 1, 2019 at 2:45 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

This is a bit more complex with 5.5. A stable 5.5 is essentially the end of the branch plus 2 bug fixes that came in the following ~2 weeks. One is a memory leak, another broken compile on Windows. I'll explain in a ticket. I need to find them first.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-478697968, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu5GWDLWAa6bcHB7CUGaKdR8PODSlks5vclPPgaJpZM4P8sCe .

kkm000 commented 5 years ago

Oh. I am nor really sure now. I've seen a huge branch merge at c0b18b77b on 2018-09-03, EDIT after right before which the current version was changed to 5.5. Our tools report 5.5, since that is in the version file, and my understanding was this is what indicates the current version. I am talking about this graph (the tag 'apparent-5.5' I added locally):

* 1669d2401 [egs,scripts] chime-4 advanced baseline (#2142)
*   a0bc18edc [src] Remove kaldi-gpsr.{h,cc} which was not used.
|\
| *   c0b18b77b resolved conflicts
| |\
| |/
|/|
* |   f0a793112 [src] Fix remaining warnings caused by #2411
|\ \
| * | 9f0db6374 (tag: apparent-5.5) Two more small fixes.
| * | 8d8c5af55 Fix style errors.
| * | 0621b792b Fix remaining -Wmaybe-unitialized warnings.
|/ /
* |   bda1dc7cf [src,scripts,egs] Grammar decoding; upgrade version to 5.5.
|\ \
| * | bdcdd4722 [doc] Update version documentation for version 5.5.
| * | 7aab92b7c [build] Upgrade version to 5.5
| * | 8b0bc2b89 [src] Fix leak in make-grammar-fst
| * | 074a1d9eb [src,egs] Cosmetic changes, mostly fixes to comments.
| * | 5cd7cde59 [scripts] Small fix in grammar-decoding script
| * | 03b585468 [doc] update grammar-fst documentation
| * | d0c68a60e [src] Refactor online decoder; get grammar decoding work in online case.
| * | a39b15c2b [egs] Small example script fixes
kkm000 commented 5 years ago

So it looked like there was a big merge before the version was changed, as if it was waiting until the trunk version stabilizes. But there are 2 essential fixes made to it later.

danpovey commented 5 years ago

I am OK to take those fixes, merge them into the 5.4 branch, and move the tag forward.

kkm000 commented 5 years ago

But what does the version signify? What does this change in the master branch mean: the development of the version 5.5 started from this point on, or was this the release of 5.5?

$ git show 7aab92b7c
commit 7aab92b7c2cca88875a92312dd4793c737ad7f03
Author: Daniel Povey <dpovey@gmail.com>
Date:   Sun Sep 2 13:15:35 2018 -0400

    [build] Upgrade version to 5.5

diff --git a/src/.version b/src/.version
index 37c2d9960..9ad974f61 100644
--- a/src/.version
+++ b/src/.version
@@ -1 +1 @@
-5.4
+5.5
danpovey commented 5 years ago

That is the start of branch 5.5: would be equivalent to 5.5.0.

the tag 5.4 indicates the end of branch 5.4, e.g. 5.4.big-number

On Mon, Apr 1, 2019 at 6:38 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

But what does the version signify? What does this change in the master branch mean: the development of the version 5.5 started from this point on, or was this the release of 5.5?

$ git show 7aab92b7c commit 7aab92b7c2cca88875a92312dd4793c737ad7f03 Author: Daniel Povey dpovey@gmail.com Date: Sun Sep 2 13:15:35 2018 -0400

[build] Upgrade version to 5.5

diff --git a/src/.version b/src/.version index 37c2d9960..9ad974f61 100644--- a/src/.version+++ b/src/.version@@ -1 +1 @@-5.4+5.5

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-478772761, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVuwhRAIOVys85XU6nKcyDJ0VSGszXks5vcop_gaJpZM4P8sCe .

kkm000 commented 5 years ago

Well, we have no tags, only branches. There is a 5.4 branch, but it has not advanced off master (has no commits not in master, directly reachable from the head of master).

So we essentially do have a release 5.5, right?

danpovey commented 5 years ago

OK, branches then, not tags. As far as I am concerned there is no concept of releases. If you want you can apply fixes to the older branches like 5.4. That's why I created those branches-- to record the last working/compatible state of those version numbers and allow the opportunity for small patches to be added to them.

On Mon, Apr 1, 2019 at 6:45 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

Well, we have no tags, only branches. There is a 5.4 branch, but it has not advanced off master (has no commits not in master, directly reachable from the head of master).

So we essentially do have a release 5.5, right?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-478774086, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu69qCuMB4_NA069d3OX_nuZFkcX-ks5vcovwgaJpZM4P8sCe .

kkm000 commented 5 years ago

Ok. Now I'm lost thoroughly, but I do not think this is important, really. I'll leave it as it is.

I think I also misunderstood the @vince62s's concern. So the doc page essentially just a git log, and the users would rather see a list of major functional changes only. I think I'll leave the issue open then, but I do not really know how to even approach this.

vince62s commented 5 years ago

@kkm000 My comment is quite old and Dan obviously did not want to maintain the "release concept" in github. Another example is here https://github.com/OpenNMT/OpenNMT-py/releases It's just a link between an arbitrary ralease number and a tag. Same concept as Dan's but in Github's world.

danpovey commented 5 years ago

I don't want to go that route because it might steer people towards using older versions of Kaldi, which would increase maintenance workload, since we would have to keep track of which bugs exist in older versions.

On Tue, Apr 2, 2019 at 4:01 AM Vincent Nguyen notifications@github.com wrote:

@kkm000 https://github.com/kkm000 My comment is quite old and Dan obviously did not want to maintain the "release concept" in github. Another example is here https://github.com/OpenNMT/OpenNMT-py/releases It's just a link between an arbitrary ralease number and a tag. Same concept as Dan's but in Github's world.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-478888186, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu5qWIVp0PSXpeOHvBu5awE8PAD0Mks5vcw5QgaJpZM4P8sCe .

kkm000 commented 5 years ago

I think the main concern here is that the version log http://kaldi-asr.org/doc/versions.html is too fine-grained. For example, this is a "patch release" version:

5.5.262 6134c290f 2019-03-20 [scripts] Fix bug in comment (#3152)

and the changeset, in its entirety,

index 34476fdb3..335e69e9a 100755
--- a/egs/wsj/s5/utils/parse_options.sh
+++ b/egs/wsj/s5/utils/parse_options.sh
@@ -44,3 +44,3 @@ done
 ###
-### No we process the command line options
+### Now we process the command line options
 ###

So marking important changes does make sense, but I really have no idea how to define "important" in this sense, and how not to forget to maintain that. The latter part is the trickiest one. : )

danpovey commented 5 years ago

We could add a tag [minor] or something like that, to suppress unimportant changes from appearing there.

On Tue, Apr 2, 2019 at 8:25 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

I think the main concern here is that the version log http://kaldi-asr.org/doc/versions.html is too fine-grained. For example, this is a "patch release" version:

5.5.262 6134c29 https://github.com/kaldi-asr/kaldi/commit/6134c290fd58680785bdcd6aa1368be045c620eb 2019-03-20 [scripts] Fix bug in comment (#3152 https://github.com/kaldi-asr/kaldi/pull/3152)

and the changeset, in its entirety,

index 34476fdb3..335e69e9a 100755--- a/egs/wsj/s5/utils/parse_options.sh+++ b/egs/wsj/s5/utils/parse_options.sh@@ -44,3 +44,3 @@ done

-### No we process the command line options+### Now we process the command line options

So marking important changes does make sense, but I really have no idea how to define "important" in this sense, and how not to forget to maintain that. The latter part is the trickiest one. : )

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-479269597, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu_rCiJGuwV9TONdbQCcAkR5YPtR8ks5vc_UIgaJpZM4P8sCe .

danpovey commented 5 years ago

Or better, a tag [major], e.g. [src,egs,major] ... since most PRs are minor.

On Tue, Apr 2, 2019 at 8:37 PM Daniel Povey dpovey@gmail.com wrote:

We could add a tag [minor] or something like that, to suppress unimportant changes from appearing there.

On Tue, Apr 2, 2019 at 8:25 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

I think the main concern here is that the version log http://kaldi-asr.org/doc/versions.html is too fine-grained. For example, this is a "patch release" version:

5.5.262 6134c29 https://github.com/kaldi-asr/kaldi/commit/6134c290fd58680785bdcd6aa1368be045c620eb 2019-03-20 [scripts] Fix bug in comment (#3152 https://github.com/kaldi-asr/kaldi/pull/3152)

and the changeset, in its entirety,

index 34476fdb3..335e69e9a 100755--- a/egs/wsj/s5/utils/parse_options.sh+++ b/egs/wsj/s5/utils/parse_options.sh@@ -44,3 +44,3 @@ done

-### No we process the command line options+### Now we process the command line options

So marking important changes does make sense, but I really have no idea how to define "important" in this sense, and how not to forget to maintain that. The latter part is the trickiest one. : )

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-479269597, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu_rCiJGuwV9TONdbQCcAkR5YPtR8ks5vc_UIgaJpZM4P8sCe .

kkm000 commented 5 years ago

This, I believe, could be automatically tagged. I'll check what tools/bots are available for GitHub for this.

danpovey commented 5 years ago

I'd rather do it personally, since what is major or not does not have to do with how many lines of code changed.

On Tue, Apr 2, 2019 at 8:47 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

This, I believe, could be automatically tagged. I'll check what tools/bots are available for GitHub for this.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-479275613, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVuyKyloEul643wVdVu1rN3PrR829Bks5vc_o7gaJpZM4P8sCe .

kkm000 commented 5 years ago

Right, this is of course manual. What I mean is a tool/script/thing which, upon seeing the [..major..] marker, slaps a tag on it, and lists a changelog to the previous tag/marker in the release notes on GitHub.

danpovey commented 5 years ago

I am not planning to use the GitHub release mechanism, because I think it goes against the dynamics of the project.

On Tue, Apr 2, 2019 at 8:59 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

Right, this is of course manual. What I mean is a tool/script/thing which, upon seeing the [..major..] marker, slaps a tag on it, and lists a changelog to the previous tag/marker in the release notes on GitHub.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-479278958, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVuyOOp18rM5C-DBXHR_zyQwLhsJ-qks5vc_0AgaJpZM4P8sCe .

kkm000 commented 5 years ago

No, not full-blown "releases", like with the source tarball. That would not make sense, I'm totally with you. Simply tagging and adding a tag message.

But then, I do not know if this is really worth the effort. There is a lot of changes that are important, even if one-line bug fixes. I samples the 20 latest commits, and marked '(!)' those that are likely "important": either bug fixes or new features; I counted 12. It's really unsimple to decide what is [major], easier said than done. If only mark a feature, then the initial commit is usually followed by bug fixes anyway. Then the [major] becomes rather confusing.

Maybe we should change exactly nothing? :)

  1. (!) [scripts] Fix bug in extend_lang.sh regarding extra_disambig.txt (#3195)
  2. (!) [build] Add missing dependency to Makefile (#3191)
  3. [github] Add GitHub issue templates (#3187)
  4. [src] Efficiency improvement and extra checking for cudamarix, RE def…
  5. (!) [scripts] Fix non-randomness in getting utt2uniq, introduced in #3142 (…
  6. (!) [build] Add new nvidia tools to windows build (#3159)
  7. [src] Disable unget warning in PeekToken (and other small fix) (#3163)
  8. (!) [scripts] Fix bug in steps/segmentation/ali_to_targets.sh (#3155)
  9. (!) [src] Add binary that functions as a TCP server (#2938)
  10. [src] Fix typo in comment (#3147)
  11. [build] Make sure PaUtils exported from portaudio (#3144)
  12. (!) [src] Fix to "Fixes to grammar-fst & LM-disambig symbols" (#3000) (#3143
  13. (!) [src] Fix bad assert in fstmakecontextsyms (#3142)
  14. (!) [scripts] Bug-fix for removing deleted words (#3116)
  15. (!) [egs] Add "formosa_speech" recipe (Taiwanese Mandarin ASR) (#2474)
  16. [scripts] Simplify text encoding in RNNLM scripts (now only support u…
  17. [doc] Small documentation fixes; update on Kaldi history (#3031)
  18. [src,scripts] Python2 compatibility fixes and code cleanup for nnet1 (#…
  19. (!) [src] Fix wrong assertion failure in nnet3-am-compute (#3106)
  20. (!) [scripts] Fix frame_shift bug in egs/swbd/s5c/local/score_sclite_conf…
danpovey commented 5 years ago

Yeah, I think the status quo is OK.

On Tue, Apr 2, 2019 at 9:26 PM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

No, not full-blown "releases", like with the source tarball. That would not make sense, I'm totally with you. Simply tagging and adding a tag message.

But then, I do not know if this is really worth the effort. There is a lot of changes that are important, even if one-line bug fixes. I samples the 20 latest commits, and marked '(!)' those that are likely "important": either bug fixes or new features; I counted 12. It's really unsimple to decide what is [major], easier said than done. If only mark a feature, then the initial commit is usually followed by bug fixes anyway. Then the [major] becomes rather confusing.

Maybe we should change exactly nothing? :)

  1. (!) [scripts] Fix bug in extend_lang.sh regarding extra_disambig.txt (#3195 https://github.com/kaldi-asr/kaldi/pull/3195)
  2. (!) [build] Add missing dependency to Makefile (#3191 https://github.com/kaldi-asr/kaldi/pull/3191)
  3. [github] Add GitHub issue templates (#3187 https://github.com/kaldi-asr/kaldi/pull/3187)
  4. [src] Efficiency improvement and extra checking for cudamarix, RE def…
  5. (!) [scripts] Fix non-randomness in getting utt2uniq, introduced in

    3142 https://github.com/kaldi-asr/kaldi/pull/3142 (…

  6. (!) [build] Add new nvidia tools to windows build (#3159 https://github.com/kaldi-asr/kaldi/pull/3159)
  7. [src] Disable unget warning in PeekToken (and other small fix) (

    3163 https://github.com/kaldi-asr/kaldi/pull/3163)

  8. (!) [scripts] Fix bug in steps/segmentation/ali_to_targets.sh (#3155 https://github.com/kaldi-asr/kaldi/pull/3155)
  9. (!) [src] Add binary that functions as a TCP server (#2938 https://github.com/kaldi-asr/kaldi/pull/2938)
  10. [src] Fix typo in comment (#3147 https://github.com/kaldi-asr/kaldi/pull/3147)
  11. [build] Make sure PaUtils exported from portaudio (#3144 https://github.com/kaldi-asr/kaldi/pull/3144)
  12. (!) [src] Fix to "Fixes to grammar-fst & LM-disambig symbols" (

    3000 https://github.com/kaldi-asr/kaldi/pull/3000) (#3143

    https://github.com/kaldi-asr/kaldi/pull/3143

  13. (!) [src] Fix bad assert in fstmakecontextsyms (#3142 https://github.com/kaldi-asr/kaldi/pull/3142)
  14. (!) [scripts] Bug-fix for removing deleted words (#3116 https://github.com/kaldi-asr/kaldi/pull/3116)
  15. (!) [egs] Add "formosa_speech" recipe (Taiwanese Mandarin ASR) (

    2474 https://github.com/kaldi-asr/kaldi/pull/2474)

  16. [scripts] Simplify text encoding in RNNLM scripts (now only support u…
  17. [doc] Small documentation fixes; update on Kaldi history (#3031 https://github.com/kaldi-asr/kaldi/pull/3031)
  18. [src,scripts] Python2 compatibility fixes and code cleanup for nnet1 (#…
  19. (!) [src] Fix wrong assertion failure in nnet3-am-compute (#3106 https://github.com/kaldi-asr/kaldi/pull/3106)
  20. (!) [scripts] Fix frame_shift bug in egs/swbd/s5c/local/score_sclite_conf…

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/1943#issuecomment-479286573, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu_kjgRc3Jgi2X8Tzrw5QOSs8QX8sks5vdANKgaJpZM4P8sCe .

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.