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

ENH: dtype argument in random.Generator.normal #23694

Open
jamesbraza opened this issue May 2, 2023 · 6 comments
Open

ENH: dtype argument in random.Generator.normal #23694

jamesbraza opened this issue May 2, 2023 · 6 comments

Comments

@jamesbraza
Copy link

Proposed new feature or change:

It would be great to support a dtype argument in random.Generator.normal.

dtype is already supported the similar function random.Generator.standard_normal for float32 and float64 as of numpy==1.24.3. Perhaps this behavior can be easily ported over.

@DuanBoomer
Copy link

Seems like a logical path to me for enhancement.
Working on it👍

DuanBoomer added a commit to DuanBoomer/numpy that referenced this issue May 2, 2023
This adds a dtype parameter in numpy.Generator.normal.
> I compared a similar function mentioned in the issue numpy#23694 to gain some insight on adding the parameter
> Closes numpy#23694
@rkern
Copy link
Member

rkern commented May 2, 2023

The omission is deliberate, but could be reconsidered. We don't want to add dtype= flexibility to all of the methods, so we restricted it to just those that are fundamental building blocks (integers(), standard_*()) where we could make use of the reduced precision to use different algorithms. I'm not particularly opposed to extending dtype= flexibility to the methods associated with the standard_*() methods, but not particularly enthusiastic about it either.

@DuanBoomer This involves a policy choice rather than just work, so it might not be a good place for a first contribution.

@DuanBoomer
Copy link

@rkern Thank you for the clarification
I didn't knew it was a deliberate omission. I just saw the issue and tried to fix it.
Can you still take some time to just see if the changes that I made would work?

I know it won't be accepted but still would love to hear your feedback.
Please take a look at it, Thank you in advance.

@rkern
Copy link
Member

rkern commented May 2, 2023

@ngoldbaum's comment on your PR is correct. That modification does not address the feature request.

@DuanBoomer
Copy link

Thank you so much for the feedback @ngoldbaum and @rkern , I will try to improve the cpython implementation on my fork just to learn something out of it.
One last request can you please guide me how to check the "numpy.Generator.normal" functions parameters if i update them.

Example: If a real user goes and types numpy.Generator.normal he will usually see the parameters to be included in it when using a editor like VS Code.
How can I see the same thing?

@bashtage
Copy link
Contributor

bashtage commented Jun 6, 2023

normal makes use of some support functions that handle broadcasting and reshaping of inputs and size arguments. I believe these are only available for C double, and so implementing an effective C float path (one that would be 32-bit throughout, rather than 64 bit and then casting down) would require appropriate versions of these.

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

Successfully merging a pull request may close this issue.

5 participants