Skip to content

Add support for CdfNorm #56

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

Closed
RobbieMackenzie opened this issue Dec 14, 2022 · 6 comments
Closed

Add support for CdfNorm #56

RobbieMackenzie opened this issue Dec 14, 2022 · 6 comments

Comments

@RobbieMackenzie
Copy link

Hi,

Great package, thanks for making it, it's sped up my code considerably.

I'm aware this would probably come under the "if there's enough interest" category, but would it be possible to add support for VML's CdfNorm function? It's not urgent as we can just use the relation to erf() instead, but it'd be interesting to see if using the dedicated function is faster. I would offer to give it a go myself, but I'm not sure I have the chops.

Cheers

@Crown421
Copy link
Collaborator

Hey, very glad you like this package.

I think that should be doable, you mean this function, correct?
https://www.smcm.iqfr.csic.es/docs/intel/mkl/mkl_manual/vml/functn_vCdfNorm.htm

@RobbieMackenzie
Copy link
Author

Oh cool! That's the function I mean, yes.

@Crown421
Copy link
Collaborator

Crown421 commented Jan 1, 2023

Sorry that it took me a while, I made an MR and should have it merged by the end of the day. From my (not very rigorous) tests, it seems to be faster than going via erf.

@RobbieMackenzie
Copy link
Author

Thanks Steffan that's great, it looks like there was an issue with the MacOS/Julia nightlies which prevented the merge, but I'll give it a go once it's merged and let you know how it performs in my code. Happy to do some comparisons for the README if that's useful - I don't think Base has a cdfnorm() function, but I could run comparisons for various dimensions both against the IVM.erf() -based implementation and against the standard cdf() function applied to a normal distribution from Distributions.jl .

@Crown421
Copy link
Collaborator

Crown421 commented Jan 3, 2023

There was an issue with the tagbot, which only got fixed today. Will hopefully merge later today.

@Crown421
Copy link
Collaborator

Crown421 commented Jan 3, 2023

Ok, it is done, merged, and registered.

@Crown421 Crown421 closed this as completed Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants