-
Notifications
You must be signed in to change notification settings - Fork 15
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
WIP: First src/hmm wraps + module + Darwin compilation #89
base: pybind11
Are you sure you want to change the base?
Conversation
|
||
using namespace kaldi; | ||
|
||
void pybind_hmm_tree_accu(py::module& m) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
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 ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/pybind/hmm/tree-accu_pybind.cc
<#89 (comment)>:
> +//
+// 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
<#89?email_source=notifications&email_token=ACUKYX2EZAVCQQUBROO2NIDQZN3QZA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPZDNMA#pullrequestreview-334640816>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACUKYX3KJNP6QDGFIVZ3DFTQZN3QZANCNFSM4J5GA3DQ>
.
|
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 ***@***.***> 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 ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/pybind/hmm/tree-accu_pybind.cc
> <#89 (comment)>:
>
> > +//
> +// 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
> <
#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
<#89?email_source=notifications&email_token=AAZFLO3JVK45YONZJUFSSK3QZN4PPA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHJVV2Y#issuecomment-567499499>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZFLO2E25YYQD4MZPYZK5LQZN4PPANCNFSM4J5GA3DQ>
.
|
I sent question in the kaldi10... Hopefully someone will know the answer and it will be useful for other people... |
From my limited experience, any custom types not in this table need to be wrapped. Otherwise, python does not know what kind of |
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 ***@***.***> 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
<#89?email_source=notifications&email_token=ACUKYX2NRXZE6G55DYFFT3TQZOKDDA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHKBIEI#issuecomment-567546897>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACUKYXYYW6X6BLIU7GQWQJTQZOKDDANCNFSM4J5GA3DQ>
.
|
I have refactored the Makefile in this pullrequest: https://github.com/danpovey/kaldi/pull/88/files now you can use |
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 ***@***.***> 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
<#89?email_source=notifications&email_token=ACUKYXY3LQJ3ZBLCCKXJ4DDQZOOZBA5CNFSM4J5GA3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHKFJBI#issuecomment-567563397>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACUKYXZE5QSQDT5J5ODPCI3QZOOZBANCNFSM4J5GA3DQ>
.
|
The makefile infrastructure is very crude (close to nonfunctional) ATM