search

hotg-ai / rune

Rune provides containers to encapsulate and deploy edgeML pipelines and applications
Apache License 2.0
134 stars 15 forks source link

microspeech calibrate_models branch bug #117

Closed meelislootus closed 3 years ago

meelislootus commented 3 years ago

I think the issue might be i8 implementation in runic-types: this was a previous attempt of this, which we did not merge into master: https://github.com/hotg-ai/rune/pull/63/commits/ed8547b4d0d358004b6e892808f65d77227f9b57

Looks like potentially changes are required under runic-types/value.rs and runic-types/buffer.rs

Michael-F-Bryan commented 3 years ago

Was this just the rustc-serialize issue?

meelislootus commented 3 years ago

Yep that was the one that had rustc-serialize problem in the error message

Michael-F-Bryan commented 3 years ago

The root cause was because rustc-serialize (one of mel's transitive dependencies) doesn't provide impls for a certain trait when compiled to wasm32-unknown-unknown. I didn't look too deeply into it, but I think they are probably doing some conditional compilation shenanigans to use different serializing methods depending on the OS/architecture.

The dependency pulling in rustc-serialize (num) had actually resolved this issue, but we didn't pick it up because we were using really ancient versions (mel used v0.1.40 and the current version is v0.4.0).The solution was to fork the mel crate to hotg-ai/mel and bump our dependencies.

Once we were no longer pulling in rustc-serialize, I found that mel and all of its DSP-related dependencies weren't no_std, meaning it'd try to pull in the standard library whenever we compile to wasm32-unknown-unknown. Luckily most of the functionality they expose is pure code and independent of the standard library anyway, so we only needed to fork the dependencies make a couple tweaks with https://github.com/hotg-ai/apodize/commit/41baaee092a9b49e26442d946046a8c3ec3d9ccf, https://github.com/hotg-ai/hertz/commit/707d6d3c239663f04cc22f33391fbe54833189db, and in hotg-ai/mel.

Michael-F-Bryan commented 3 years ago

This has been resolved by the above solution and #107 has now been merged.