implement IsValid and IsConnected

[qindazhu]

No description provided.

[qindazhu]

It seems .clang-format file was not working.. @csukuangfj do you have any idea about this?

[qindazhu]

The test on MacOs failed (but succeeded on Ubuntu), does anyone know the reason or where can I find the detailed logs? I have no Mac device at my hand.

[danpovey]

reopening so I can see instrutions to check it out

[danpovey]

The project has no INSTALL.txt file and installation instructions aren't in the README.md... can someone please add those? I dont know how to test.

[qindazhu]

Thanks Dan. Maybe you can review the code first in case there are some issues I should fix.

[danpovey]

I skimmed it and it looked OK, but I can't test on mac without instructions.

[qindazhu]

Ok, thanks. I will write one.

[danpovey]

This was the error

[ 45%] Building CXX object k2/csrc/CMakeFiles/fsa_renderer.dir/fsa_renderer.cc.o
In file included from /Users/dpovey/k2/k2/csrc/fsa_renderer.cc:7:
In file included from /Users/dpovey/k2/k2/csrc/fsa_renderer.h:9:
/Users/dpovey/k2/k2/csrc/fsa.h:123:25: error: use of undeclared identifier 'int32'
  std::vector<std::pair<int32, int32> > elems;
                        ^
/Users/dpovey/k2/k2/csrc/fsa.h:123:39: error: expected a type
  std::vector<std::pair<int32, int32> > elems;
                                      ^
2 errors generated.
make[2]: *** [k2/csrc/CMakeFiles/fsa_renderer.dir/fsa_renderer.cc.o] Error 1
make[1]: *** [k2/csrc/CMakeFiles/fsa_renderer.dir/all] Error 2
make: *** [all] Error 2

[qindazhu]

It is strange that we only use int32_t instead of int32, why is it int32 on Mac.

[danpovey]

int32_t is what you get from #include <cstint>... int32 has to be defined manually.

…

On Sat, Apr 25, 2020 at 9:15 PM Haowen Qiu ***@***.***> wrote: It is strange that we only use int32_t instead of int32, why is it int32 on Mac. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <https://github.com/danpovey/k2/pull/10#issuecomment-619378168>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZFLOYCLP4FRV6FZUQ7WS3ROLPALANCNFSM4MQWW5ZQ> .

[qindazhu]

int32_t is what you get from #include ... int32 has to be defined manually.
…
On Sat, Apr 25, 2020 at 9:15 PM Haowen Qiu @.***> wrote: It is strange that we only use int32_t instead of int32, why is it int32 on Mac. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#10 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFLOYCLP4FRV6FZUQ7WS3ROLPALANCNFSM4MQWW5ZQ .

I mean we never define int32 or use it. I have checked fsa.h:123, it's an empty line. Dan, can you check your local file? Thanks!

[csukuangfj]

I am developing on macOS. I will test your branch.

[qindazhu]

I am developing on macOS. I will test your branch.

Thanks!

[csukuangfj]

The tests passed on my macOS.

Not sure why it fails for GitHub workflow.

$ make test
Running tests...
Test project /xxxxx/k2/build
    Start 1: Test.properties_test
1/3 Test #1: Test.properties_test .............   Passed    0.00 sec
    Start 2: Test.fsa_util_test
2/3 Test #2: Test.fsa_util_test ...............   Passed    0.00 sec
    Start 3: Test.fsa_renderer_test
3/3 Test #3: Test.fsa_renderer_test ...........   Passed    0.00 sec

100% tests passed, 0 tests failed out of 3

Total Test time (real) =   0.02 sec

$ uname -a
Darwin xxxxx 15.3.0 Darwin Kernel Version 15.3.0: Thu Dec 10 18:40:58 PST 2015; 
root:xnu-3248.30.4~1/RELEASE_X86_64 x86_64

$ gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr 
--with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.3.0
Thread model: posix

$ clang --version
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.3.0
Thread model: posix

[qindazhu]

Ok, thanks @csukuangfj . Maybe we can wait for @danpovey 's reponse as it can be reproduced on his local machine.

[csukuangfj]

Change the following line

https://github.com/danpovey/k2/blob/9a376db36af11d20d31bffafad621d45e1fc9a23/.github/workflows/build.yml#L48

to

run: CTEST_OUTPUT_ON_FAILURE=1 ctest --build-config $BUILD_TYPE 

will give you verbose output on test failure.

Maybe you can create another pullrequest to do this.

[qindazhu]

Change the following line

https://github.com/danpovey/k2/blob/9a376db36af11d20d31bffafad621d45e1fc9a23/.github/workflows/build.yml#L48

to

run: CTEST_OUTPUT_ON_FAILURE=1 ctest --build-config $BUILD_TYPE 

will give you verbose output on test failure.

Maybe you can create another pullrequest to do this.

Ok, thanks

[csukuangfj]

It seems .clang-format file was not working.. @csukuangfj do you have any idea about this?

Have you installed clang-format ?

On Linux:

pip install clang-format==9.0.0

On macOS

brew install clang-format

[qindazhu]

It seems .clang-format file was not working.. @csukuangfj do you have any idea about this?

Have you installed clang-format ?

On Linux:

pip install clang-format==9.0.0

On macOS

brew install clang-format

OK, thx!

[qindazhu]

Sad. Seems suddenly I cannot access Github on Xiaomi's clusters.....will check the build error when it's reconnected.

[qindazhu]

Ok, ready to merge.

Guys, please take a look, if there is no other issue, I'll merge it.

[csukuangfj]

Please also add CTEST_OUTPUT_ON_FAILURE=1 to build.yml.

[csukuangfj]

+1

[qindazhu]

Thanks @csukuangfj

[qindazhu]

@danpovey , seems I cannot merge this. Please help to merge, thanks!

[qindazhu]

Thanks @csukuangfj

[danpovey]

Haowen, it looks like you didn't yet accept my github invitation to you. Maybe it's in your email.

…

On Sat, Apr 25, 2020 at 10:54 PM Haowen Qiu ***@***.***> wrote: Thanks @csukuangfj <https://github.com/csukuangfj> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/danpovey/k2/pull/10#issuecomment-619391314>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZFLO22DQ7NDTNRVLQMTU3ROL2TBANCNFSM4MQWW5ZQ> .