-
Notifications
You must be signed in to change notification settings - Fork 8
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
Compile libmariandecoder to wasm #6
Conversation
…ntencepiece directory gets included when #include <version> is invoked via emscripten/system/include/libcxx)
Current compilation errors (click to expand)
|
Current compilation errors (click to expand)
|
* master: (29 commits) Compile guards for CPU and GPU only builds Fix marian conv by taking it from intgemm_reintegrated clipValue error Fix windows compiler warnings Update to latest intgemm version Update intgemm Merged, slow as hell Move batched GEMM back (marian-nmt#710) Fix MPI on GCC8+ (marian-nmt#742) Fixes bug for certain reductions (marian-nmt#746) Updates to GitHub CI workflows (marian-nmt#730) Release workflows (marian-nmt#731) Update VERSION Remove duplicated EXT_LIBS (marian-nmt#728) Old CMAKE does not understand VERSION_EQUAL_GREATER suppress sentencepiece warnings for g++ > 8.0 Merged PR 14474: CMake build fixes for QuickSAND Compute75 for NCCL (marian-nmt#711) Enable final stack post-processing for transformer for correct prenorm behavior (marian-nmt#719) fix compilation with -DCOMPILE_CPU=off ... # Conflicts: # src/3rd_party/CMakeLists.txt # src/CMakeLists.txt
Current status: Something in marian's root CMake config is interfering with emscriptens environment. Building sentencepiece directly in its 3rd_party subfolder using emscripten works (
...but when it is invoked via marian's CMake (
This showed the same symptoms, but those were removed by removing files named "VERSION" in the include path (done in 7570ef2 and 7aea659 above). Something interferes with emscriptens environment, possibly making the code include the system's stddef.h rather than emscripten's, but I don't see where this is happening. Also applied the advice from https://stackoverflow.com/a/54386659/682317 in 8326954 but it did not help. |
@motin do a |
I'm currently not able to run To make things work on my environment, I had to resurrect a few things from my marian-docker setup (most notabley: run things in the Docker container with the same user id and group id as I have on the host); to be able to build both on the host and in the Docker container, I also put back some git path mount hacks do deal with git submodule issues. You'll find these changes in branch ug-compile-wasm, which branches off of compile-wasm.
|
…ressed in another manner)
@ugermann I see, that looks like a permission error:
On my docker setup I can step into a container ( A workaround can be to copy the source code into the docker image and then build without any mounts, that should make the docker approach more portable. |
@XapaJIaMnu A yes, good suggestion. Here is log output from the successful sentencepiece compilation:
And an unsuccessful one (reproduced with the help from the verbose make output):
Full compilation output (click to expand)
|
The different configurations lead to very different compiler commands: The working setup uses:
(Which results in a successful compilation, with nothing printed to stdout or stderr) And the marian root CMake setup results in:
|
@@ -99,11 +99,15 @@ class OutputPrinter { | |||
std::string getWordScores(const Hypothesis::PtrType& hyp); | |||
|
|||
float getAlignmentThreshold(const std::string& str) { | |||
#if WITHOUT_EXCEPTIONS | |||
if (str.begin() == str.end()) return 0.f; | |||
#else |
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.
You can remove pre-processor here. The logic of if (str.begin() == str.end())
applies for exception friendly code as well: there is no need to throw exception if you know string is empty. As a rule, a throw is always times slower than just if-check. Also, you will need that std::stof
in the exception-free code.
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.
or return str.begin() == str.end() ? 0.f : std::max(std::stof(str), 0.f);
- All try/catch of marian source are guarded now - It excludes any 3rd_party library
- Few 3rd_party sources are yet to be done
- This includes changes for shuffle pattern given here: https://bugzilla.mozilla.org/show_bug.cgi?id=1672160
Hey folks. I was trying to run float 32-bit inference in WASM using
Could any one of you provide some hint what might be wrong in this case? We did disable few things in order to compile marian to wasm (e.g. zstr). The following line of the log PS: I generated model.bin from model.npz using the command: |
Updated benchmark report based on the latest code. The 5 different setups: Benchmark 7 C without --int8: 16.6s for 5644 words = 339 wps B/A: 1.0x slower Interestingly enough, intgemm inference on Chrome (or on Nightly without the wormhole enabled but with --int8 added to the benchmark command) results in slightly (appr 10-20%) slower than usual inferences times and with all translations ending up as strings of varying lengths of repeated "-" characters. Note that these numbers are not included above. Also interesting is that the performance of the current code is performing much worse on Chrome than it did previously. Note that native benchmarks are using |
Do I infer correctly that "C" at 9.7s is now the 8-bit wormhole? |
Yes. And with the updated comment it should be clearer. The wormhole / intgemm resulted in 70% higher wps in this benchmark. |
It's pointless to measure words per second with corrupt output. Because often these just loop until they hit max sentence length. And decoding time is proportionate to sentence length. Can you clearly separate the ones with corrupt output? |
Those numbers were excluded. I have updated the comment to clarify this. |
So the wormhole is working but compiling raw |
Indeed, if (a) the wormhole produces correct output and (b) the _mm_maddubs etc code produces correct output in Chrome but not in Firefox then I'd love to know what the inputs are that make it fail so that I can try to track down the problem. |
@motin Is following the set up for the behavior above? |
I clarified the original comment with the setups: C. WASM in Firefox Nightly with 8-bit wormhole enabled and --int8 added to WASM benchmark commands |
All WASM code benchmarked was compiled with wormhole instructions. |
I know the confusion here. "8-bit wormhole enabled" can mean 2 things to me (and probably to all of us):
When you say "8-bit wormhole enabled", I think you mean
I can't explain the repeated "-" characters behavior. But I believe it might be a side-effect. |
Thanks for the clarification, it makes it easy to explain what happens when :)
Exactly these two configurations are the ones that result in "-" of varying length as translation results. A pretty peculiar side effect :) |
Wrong output just means the GEMM operations are not working properly and producing some random numbers. That's the "side effect" |
Yeah it's the typical failure mode when GEMM is producing wrong answers. Nothing to see here, the wormhole instructions only work with the weird firefox configuration that supports them. |
@abhi-agg Surely we can close this PR now since it has been superseeded by more recent PRs? |
@motin Let's not close this PR just now until I merge all changes to main branch. I will close it myself once the migration is complete 👍 |
Description
This PR adds WASM builds of marian-decoder
It is related to issues: #5
List of changes:
Added dependencies: Docker
How to test
Expected results during draft stage of this PR: The compilation fails on Windows
Checklist