Fix simple problem with cproj() pointed out by Damian #19142
Conversation
In discussing math stuff recently, Damian pointed out that Chapel's cproj() function returned (and assumed C was returning) reals/floats rather than complexes. This makes the simple update to address that. (Damian also suggested that cproj() is so simple that we ought to implement it in Chapel (from a simplicity / optimizability of code perspective), but I did not feel I had the time to take that up late on a Friday. Someone should open an issue about this or take it on themselves if they feel strongly about it. --- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
|
@daviditen : Would you review? (@damianmoz is very welcome to as well, of course). |
|
I think for the moment, the simplest thing to do is just fix the bug which I think means changing inline proc cproj(z: complex(?w)): real(w/2) {to inline proc cproj(z: complex(?w)) {Minimal work. Bed that down. Bug gone away. Problem solved. Leave any rewriting in Chapel of this effectively single statement routine until after some this Math library changes go through. Or incorporate the changes as part of that process. My 2c. Your call really. |
@damianmoz : I'm not understanding your comment. Isn't that essentially what I've done in this pull request? (click on the "Files changed" tab to see the diff). |
|
Sorry, total novice here. I have looked at a pull request once before in my life. I did not realise that you had already made the changes. I just clicked on the files changed. Learnt a new skill today. Must be a good day Yes, I agree with everything you have done. Looks good. Sorry for the extraneous noise. |
In discussing math stuff recently, @damianmoz pointed out that Chapel's
cproj() function returned (and assumed C was returning) reals/floats
rather than complexes. This makes the simple update to address that.
Though the imaginary part is always 0.0, Damian points out that it can
matter since it should retain the sign of the original value (+0.0 or -0.0).
And also that users expect these routines to be closed on the input type.
Damian also suggested that cproj() is so simple that we ought to
implement it in Chapel (from a simplicity / optimizability of code
perspective), but I did not feel I had the time or energy to take that up late
on a Friday. Someone should open an issue about this or take it on
themselves if they feel strongly about it.