Skip to content
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

Rewrite f32.exp and f32.tanh? #2828

Closed
makotokato opened this issue Nov 16, 2022 · 7 comments · Fixed by #2845
Closed

Rewrite f32.exp and f32.tanh? #2828

makotokato opened this issue Nov 16, 2022 · 7 comments · Fixed by #2845
Labels
C-segmentation Component: Segmentation

Comments

@makotokato
Copy link
Member

makotokato commented Nov 16, 2022

lstm uses f32.exp and f32.tanh, so we depends on num_trait with std. Should we remove num_trait and std dependency, then rewrite f32.exp and f32.tanh?

@makotokato makotokato added the C-segmentation Component: Segmentation label Nov 16, 2022
@aethanyc
Copy link
Contributor

cc @sffc

@sffc
Copy link
Member

sffc commented Nov 16, 2022

CC @younies

@robertbastian
Copy link
Member

num_traits seems to be unused currently. However it can provide f32::exp and f32::tanh (through its Float extension trait) in #[no_std] if we enable the libm feature. Alternatively we could use libm directly, but it's less ergonomic and num_traits is a transitive dependency already.

@robertbastian
Copy link
Member

I believe the no-stdness of LSTM was the reason it's still behind a feature and not included in the meta crate. Should we include it now?

@sffc
Copy link
Member

sffc commented Dec 1, 2022

I believe the no-stdness of LSTM was the reason it's still behind a feature and not included in the meta crate. Should we include it now?

It's one of several blockers; see the meta-issue #2259

@sffc sffc mentioned this issue Dec 1, 2022
22 tasks
@robertbastian
Copy link
Member

Those are blockers for stabilizing the whole crate. I'm talking about LSTM, which does not need to be behind a feature anymore (before it had to be because otherwise icu with experimental features wouldn't have been no_std). Unless we want to keep it behind a feature even after stabilization?

@sffc
Copy link
Member

sffc commented Dec 1, 2022

The LSTM features are kind-of a mess right now. Enabling LSTM disables Dictionary. That's one of the things that needs to be fixed before stabilizing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-segmentation Component: Segmentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants