kaldi icon indicating copy to clipboard operation
kaldi copied to clipboard

WIP: First src/hmm wraps + module + Darwin compilation

Open jtrmal opened this issue 5 years ago • 7 comments

The makefile infrastructure is very crude (close to nonfunctional) ATM

jtrmal avatar Dec 19 '19 13:12 jtrmal

I will do more on this... I'm trying to figure out some stuff and this looked like an easy part to start with. What I'm currently not sure is if we need to wrap all types that appear as parameters to a function call somewhere or they will get mapped to some auxiliary handle-like type automatically by pybind y.

On Thu, Dec 19, 2019 at 2:47 PM Daniel Povey [email protected] wrote:

@danpovey commented on this pull request.

In src/pybind/hmm/tree-accu_pybind.cc https://github.com/danpovey/kaldi/pull/89#discussion_r359866996:

+// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// THIS CODE IS PROVIDED AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY IMPLIED +// WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, +// MERCHANTABLITY OR NON-INFRINGEMENT. +// See the Apache 2 License for the specific language governing permissions and +// limitations under the License.

+#include "hmm/tree-accu_pybind.h" +#include "hmm/tree-accu.h" + +using namespace kaldi; + +void pybind_hmm_tree_accu(py::module& m) {

Thanks. Do you want me to merge this soon or wait? BTW, it's not clear to me that we'd need this part of the code anytime soon.. I would start with tree, hmm, transition-model, posteriors, and functions that operate on them. Incidentally, in the kaldi10 branch I made some nice simplifications to the tree, HMM, transition-model code; but it never seems to be a good time to merge. (Breaks back compatibility).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danpovey/kaldi/pull/89?email_source=notifications&email_token=ACUKYX2EZAVCQQUBROO2NIDQZN3QZA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPZDNMA#pullrequestreview-334640816, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACUKYX3KJNP6QDGFIVZ3DFTQZN3QZANCNFSM4J5GA3DQ .

jtrmal avatar Dec 19 '19 13:12 jtrmal

My impression was that all non-inbuilt types have to be wrapped. Not sure about STL types, whether pybind11 "comes with them".

On Thu, Dec 19, 2019 at 9:56 PM jtrmal [email protected] wrote:

I will do more on this... I'm trying to figure out some stuff and this looked like an easy part to start with. What I'm currently not sure is if we need to wrap all types that appear as parameters to a function call somewhere or they will get mapped to some auxiliary handle-like type automatically by pybind y.

On Thu, Dec 19, 2019 at 2:47 PM Daniel Povey [email protected] wrote:

@danpovey commented on this pull request.

In src/pybind/hmm/tree-accu_pybind.cc https://github.com/danpovey/kaldi/pull/89#discussion_r359866996:

+// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// THIS CODE IS PROVIDED AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY IMPLIED +// WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, +// MERCHANTABLITY OR NON-INFRINGEMENT. +// See the Apache 2 License for the specific language governing permissions and +// limitations under the License.

+#include "hmm/tree-accu_pybind.h" +#include "hmm/tree-accu.h" + +using namespace kaldi; + +void pybind_hmm_tree_accu(py::module& m) {

Thanks. Do you want me to merge this soon or wait? BTW, it's not clear to me that we'd need this part of the code anytime soon.. I would start with tree, hmm, transition-model, posteriors, and functions that operate on them. Incidentally, in the kaldi10 branch I made some nice simplifications to the tree, HMM, transition-model code; but it never seems to be a good time to merge. (Breaks back compatibility).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/danpovey/kaldi/pull/89?email_source=notifications&email_token=ACUKYX2EZAVCQQUBROO2NIDQZN3QZA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPZDNMA#pullrequestreview-334640816 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/ACUKYX3KJNP6QDGFIVZ3DFTQZN3QZANCNFSM4J5GA3DQ

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/danpovey/kaldi/pull/89?email_source=notifications&email_token=AAZFLO3JVK45YONZJUFSSK3QZN4PPA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHJVV2Y#issuecomment-567499499, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFLO2E25YYQD4MZPYZK5LQZN4PPANCNFSM4J5GA3DQ .

danpovey avatar Dec 19 '19 14:12 danpovey

I sent question in the kaldi10... Hopefully someone will know the answer and it will be useful for other people...

jtrmal avatar Dec 19 '19 14:12 jtrmal

From my limited experience, any custom types not in this table need to be wrapped. Otherwise, python does not know what kind of object it is.

naxingyu avatar Dec 19 '19 15:12 naxingyu

I assume we can make them completely dummy wrappers, such as

py::class_<TransitionModel>(m, "TransitionModel")
  ;

On Thu, Dec 19, 2019 at 4:52 PM Xingyu Na [email protected] wrote:

From my limited experience, any custom types not in this table https://pybind11.readthedocs.io/en/stable/advanced/cast/overview.html#list-of-all-builtin-conversions need to be wrapped. Otherwise, python does not know what kind of object it is.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danpovey/kaldi/pull/89?email_source=notifications&email_token=ACUKYX2NRXZE6G55DYFFT3TQZOKDDA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHKBIEI#issuecomment-567546897, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACUKYXYYW6X6BLIU7GQWQJTQZOKDDANCNFSM4J5GA3DQ .

jtrmal avatar Dec 19 '19 16:12 jtrmal

I have refactored the Makefile in this pullrequest: https://github.com/danpovey/kaldi/pull/88/files

now you can use make -j and you do not need to recompile all .cc files if you made changes only to one cc file.

csukuangfj avatar Dec 19 '19 16:12 csukuangfj

thanks, I will merge it with my changes once your's PR will get merged. y.

On Thu, Dec 19, 2019 at 5:32 PM Fangjun Kuang [email protected] wrote:

I have refactored the Makefile in this pullrequest: https://github.com/danpovey/kaldi/pull/88/files

now you can use make -j and you do not need to recompile all .cc files if you made changes only to one cc file.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danpovey/kaldi/pull/89?email_source=notifications&email_token=ACUKYXY3LQJ3ZBLCCKXJ4DDQZOOZBA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHKFJBI#issuecomment-567563397, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACUKYXZE5QSQDT5J5ODPCI3QZOOZBANCNFSM4J5GA3DQ .

jtrmal avatar Dec 19 '19 17:12 jtrmal