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

Adding support for Lambert W function? #14

Closed
JSorngard opened this issue Aug 3, 2024 · 20 comments
Closed

Adding support for Lambert W function? #14

JSorngard opened this issue Aug 3, 2024 · 20 comments

Comments

@JSorngard
Copy link
Contributor

I recently finished an implementation of the method of Toshio Fukushima for evaluating the Lambert W function quickly and accurately.

Would it be interesting to integrate this functionality here?

@IvanUkhov
Copy link
Member

Hello, in general, the more, the merrier. I am just worried about how you would like to add it here. Do you propose to add a dependency on that crate or reinstantiate here?

@JSorngard
Copy link
Contributor Author

JSorngard commented Aug 5, 2024

Initially I thought adding a dependency would probably be simpler, but I see this crate is no_std through libm, while mine still unfortunately needs the standard library.

@IvanUkhov
Copy link
Member

Yeah, there is this complication. Speaking of std, I would assume you do not really need it in your crate, as it is math. Not for the sake of this crate but in general, disposing of it would make lambert_w more accessible.

@JSorngard
Copy link
Contributor Author

Yeah, it would add a dependency though, and I don't know how to judge that trade-off. But it would make the crate much more accessible.

I am currently trying to make std optional by having a std feature. The only problem is that enabling that feature would require disabling the libm feature in the one optional dependency of the crate (in order to not have unnecessary dependencies), and I don't think a feature can disable another one. I could have a separate std and libm feature and just have compilation fail when both are off, but I don't know if that is a good idea.

@JSorngard
Copy link
Contributor Author

Sorry for asking you about my own crate on here actually.

Would it be interesting to reinstantiate the code in this crate with a reference to Fukushima's paper and my crate?

@IvanUkhov
Copy link
Member

I am currently trying to make std optional by having a std feature.

Yeah, I do not think it is possible either. Perhaps you can do the other way around by having no_std feature?

Would it be interesting to reinstantiate the code in this crate with a reference to Fukushima's paper and my crate?

Well, as much as I want this crate to be independent, it would create duplication in the ecosystem. Someone would have to maintain it in two places.

@JSorngard
Copy link
Contributor Author

If I manage a good no_std solution, would using it here and re-exporting it be interesting?

@IvanUkhov
Copy link
Member

Yes, I think it would be logical to just add a dependency, as long as that dependency is minimal. But I think the interface would have to be different here to play along with the other special functions, that is, to introduce a trait (Omega?).

@JSorngard
Copy link
Contributor Author

Yeah, I think either LambertW, Omega, or ProductLog might be good names, though first I'll get to work on no_std!

@JSorngard
Copy link
Contributor Author

JSorngard commented Aug 6, 2024

My current solution for no_std is to have a std feature and a libm feature (as that is how num-traits and thus the optional dependency has structured its features). If neither is enabled it's a compile error, if std is enabled it uses the standard library, otherwise it uses libm.

The default state would be to have std disabled and libm enabled, so this crate could just import it. How does this sound?

@IvanUkhov
Copy link
Member

The problem with default features is that they cannot be opted out from in dependent crates: rust-lang/cargo#2823. So if libm is enabled by default in lambert_w, any other crate having it as a dependency will make it impossible for its own users to get rid of libm, unless that other crate provides its own features for controlling lambert_w.

In the case of special, I suppose it does not matter, as it is always no_std with libm.

@JSorngard
Copy link
Contributor Author

Yeah, that's the problem I'm running into with my own dependencies, and is why this was the only solution I could come up with.
I could make none of the features default, but that would make the crate uncompilable unless the user enables a feature.

@IvanUkhov
Copy link
Member

Yeah, having none enabled is not user friendly either.

But what is the problem with having a no_std instead of an std feature? It will plug in libm upon request.

And in general, whey do you need std in the first place? Where do you use it?

@JSorngard
Copy link
Contributor Author

JSorngard commented Aug 6, 2024

I don't really need the standard library, but the crate is a direct dependency in other crates that do use it and want to minimize the dependency tree, so I want to provide the option.

I don't think having only a no_std feature would work because enabling it would have to disable the std feature in an optional dependency, and as far as I know that is not possible.

@IvanUkhov
Copy link
Member

Another reason for keeping the standard library around is that, since libm is a reimplementation restricted to core, it can be slower than std in some cases, as it can leverage the hardware more efficiently via specialized instructions. So one is needed for portability, such as when targeting WebAssembly, while the other one is needed for speed.

@IvanUkhov
Copy link
Member

But then I would just mimic the feature structure in the dependency, as you proposed above. The dependent crates will have to deal with it themselves.

@JSorngard
Copy link
Contributor Author

JSorngard commented Aug 6, 2024

I've made the crate no_std by adding the two features: https://docs.rs/lambert_w/0.5.2/lambert_w/

@IvanUkhov
Copy link
Member

Great work! By the way, did you by any change compare the performance of std with that of libm? I see it is only two functions used; so probably not much of a difference in the end.

If you would like to try to expose it via special, I would propose to stick to only one of the accuracy version, probably the 50-bit one. Since all other traits here are Greek letters, it is really tempting to call it Omega, but I understand it might be less conventional. Another complication is that each trait is implemented for not only f64 but also f32, but I suppose f32 can be skipped until later.

@JSorngard
Copy link
Contributor Author

I did compare them! The standard library makes the functions around 10 to 20% faster on random inputs (I haven't benchmarked fixed inputs yet).

Then it makes sense to use omega, quite satisfying. I agree on using the 50-bit versions. While the 24-bit versions are faster they are only around 5 to 15% faster.

I have been thinking about converting the 24-bit versions into using f32, since that's the size of their mantissa. But that would require a rewrite and a breaking change, so I think beginning with just the 50-bit versions for f64 is a good idea.

@IvanUkhov
Copy link
Member

IvanUkhov commented Aug 8, 2024

That is a significant difference. Just pushed support for std (where it was possible). The current benchmark do not show much of a difference except for trigamma:

https://github.com/stainless-steel/special/actions/runs/10300025351/job/28508574173

Might be good to keep it anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants