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

Change log / Release #1943

Open
vince62s opened this issue Oct 17, 2017 · 26 comments
Open

Change log / Release #1943

vince62s opened this issue Oct 17, 2017 · 26 comments
Assignees
Labels
stale Stale bot on the loose

Comments

@vince62s
Copy link
Contributor

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:https://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
Copy link
Contributor

danpovey commented Oct 17, 2017 via email

@vince62s
Copy link
Contributor Author

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
Copy link
Contributor

kkm000 commented Apr 1, 2019

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.

@kkm000 kkm000 closed this as completed Apr 1, 2019
@danpovey
Copy link
Contributor

danpovey commented Apr 1, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 1, 2019

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
Copy link
Contributor

danpovey commented Apr 1, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 1, 2019

Oh. I am nor really sure now. I've seen a huge branch merge at c0b18b7 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
Copy link
Contributor

kkm000 commented Apr 1, 2019

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
Copy link
Contributor

danpovey commented Apr 1, 2019

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

@kkm000
Copy link
Contributor

kkm000 commented Apr 1, 2019

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 <[email protected]>
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
Copy link
Contributor

danpovey commented Apr 1, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 1, 2019

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
Copy link
Contributor

danpovey commented Apr 1, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 1, 2019

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.

@kkm000 kkm000 reopened this Apr 1, 2019
@kkm000 kkm000 self-assigned this Apr 1, 2019
@vince62s
Copy link
Contributor Author

vince62s commented Apr 2, 2019

@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
Copy link
Contributor

danpovey commented Apr 2, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 3, 2019

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

5.5.262 6134c29 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
Copy link
Contributor

danpovey commented Apr 3, 2019 via email

@danpovey
Copy link
Contributor

danpovey commented Apr 3, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 3, 2019

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

@danpovey
Copy link
Contributor

danpovey commented Apr 3, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 3, 2019

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
Copy link
Contributor

danpovey commented Apr 3, 2019 via email

@kkm000
Copy link
Contributor

kkm000 commented Apr 3, 2019

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 (minor bug (phones.txt instead of words.txt) #3195)
  2. (!) [build] Add missing dependency to Makefile ([build] Add missing dependency to Makefile #3191)
  3. [github] Add GitHub issue templates (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 [src] Fix wrong assertion failure in fstmakecontextsym #3142 (…
  6. (!) [build] Add new nvidia tools to windows build ([WIP] Windows cub building #3159)
  7. [src] Disable unget warning in PeekToken (and other small fix) ([src] Disable unget warning in PeekToken #3163)
  8. (!) [scripts] Fix bug in steps/segmentation/ali_to_targets.sh (Debugged syntax bug in *steps/segmentation/ali_to_targets.sh* #3155)
  9. (!) [src] Add binary that functions as a TCP server (Online2 NNet3 TCP server program #2938)
  10. [src] Fix typo in comment ([doc] fix an error in the comment for "LevenshteinEditDistance()" #3147)
  11. [build] Make sure PaUtils exported from portaudio (make sure you export PaUtils when compiling portaudio during make ext #3144)
  12. (!) [src] Fix to "Fixes to grammar-fst & LM-disambig symbols" ([src] Fixes to grammar-fst code to handle LM-disambig symbols properly #3000) ([src] Fix to "Fixes to grammar-fst & LM-disambig symbols" (#3000) #3143
  13. (!) [src] Fix bad assert in fstmakecontextsyms ([src] Fix wrong assertion failure in fstmakecontextsym #3142)
  14. (!) [scripts] Bug-fix for removing deleted words ([scripts] Bug-fix for removing deleted words #3116)
  15. (!) [egs] Add "formosa_speech" recipe (Taiwanese Mandarin ASR) ("formosa_speech" recipe and database for Taiwanese Mandarin speech recognition #2474)
  16. [scripts] Simplify text encoding in RNNLM scripts (now only support u…
  17. [doc] Small documentation fixes; update on Kaldi history (update the docs: #3031)
  18. [src,scripts] Python2 compatibility fixes and code cleanup for nnet1 (#…
  19. (!) [src] Fix wrong assertion failure in nnet3-am-compute (Failed assertion only if divide by priors is set to True. #3106)
  20. (!) [scripts] Fix frame_shift bug in egs/swbd/s5c/local/score_sclite_conf…

@danpovey
Copy link
Contributor

danpovey commented Apr 3, 2019 via email

@stale
Copy link

stale bot commented Jun 19, 2020

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.

@stale stale bot added the stale Stale bot on the loose label Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale bot on the loose
Projects
None yet
Development

No branches or pull requests

3 participants