This is currently very elegantly coded as
inline proc min(x: real(?w), y: real(w)) return if (x < y) | isnan(x) then x else y;
The following might be better:
inline proc min(x: real(?w), y: real(w)) return if (x < y) | (x != x) then x else y;
as it is technically more correct and just as elegant. The only reason that the isnan function was introduced into the IEEE 754 interface is that some overly aggressive optimizing compilers (mainly in the past) have deluded themselves that the statement
x != x
is always false when it is actually true (by definition) when the variable
x is a Not-a-Number or NaN in IEEE 754 speak.
The Chapel compiler is certainly not one of delusional compilers when it comes to floating point It understands the concept properly and handles the statement correctly.
On some hardware, it may be slightly slower, on some slightly faster. On my limited testing on current Intel, it is much the same. On older Intel it was slightly slower as it demanded an extra memory access. I will try and do some testing on ARM later and might test some equivalernt C on IBM Power 8 unless someone tells me that Chapel will compile on that architecture.
for what is worth,
x != x is indeed how
isnan is defined in some languages (at least Julia and Rust). With some detective work, it seems chapel calls the C-function
isnan? (see Chapel source code here and here)
Not sure what the C-implementation does under the hood, but the two versions do seem to generate slightly different assembly (at least with some compilers on some architectures): see this example (although to be fair I don't feel educated enough to compare the assembly versions)
What would you think about opening a GitHub PR that implements this proposed code change? That would probably be the best way to make sure it doesn't end up in limbo here on a Discussion topic (where the next best way would be to open a GitHub issue proposing the change... but as you know, there are a lot more open issues than open PRs).
Sure. I deliberately did not do Github because I was unsure whether changing this to my suggestion is optimal. But sure. Happy to move it there.
Well, I logged into Github, went ...
Create Issue but the Submit New Issue button is greyed out? Am I in the naughty corner?
Damian -- could you confirm that you are at https://github.com/chapel-lang/chapel/issues/new and that you have given your issue a title?
Looks like I forgot to switch my brain on - Sorry.
@damianmoz - I see you made this GitHub issue -- Maximum/Minimum of Two Real Numbers · Issue #20390 · chapel-lang/chapel · GitHub . I wanted to ask if you had intentionally opted not to create a Pull Request (which Brad was suggesting) or if there was some mis-communication.
You can see the list of current Pull Requests here: Pull requests · chapel-lang/chapel · GitHub . The Pull Request would include the source code changes you are proposing so that we can test them and when we merge it, give you credit for them in the commit history. Also, we have some documentation on how to get to a first PR here: Contributor Info — Chapel Documentation 1.27 .
I did not realise Brad was talking about a PR. Not his miscommunication. My lack of understanding.
That said, I was more interested in some discussion about whether my suggested approach is sound. My ideas could be ill advised.
I don't think there's a problem here: I did offer up both approaches. I don't feel like I'm expert enough personally to offer an opinion on the merits of the proposal (or I'd have commented on the issue).