[go: nahoru, domu]

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

important numerical issue with (at least) SQRT() & ALOG() for negative inputs (due to change in IDL 8.5.1 in NaN for MIN/MAX) #1353

Open
alaingdl opened this issue Aug 6, 2022 · 5 comments
Assignees

Comments

@alaingdl
Copy link
Contributor
alaingdl commented Aug 6, 2022

And going back in the past all GDL versions tested since 1.0.0 in august 2021 are buggy

GDL> rr=findgen(1000)-10
GDL> ee=alog(rr)
GDL> print, min(ee), max(ee)
         -NaN         -NaN
GDL> ee=sqrt(rr)
GDL> print, min(ee), max(ee)
         -NaN         -NaN

but

IDL> !version.release
8.8.0
IDL> rr=findgen(1000)-10
IDL> ee=sqrt(rr)
% Program caused arithmetic error: Floating illegal operand
IDL> print, min(ee), max(ee)
      0.00000      31.4484
IDL> ee=alog(rr)
% Program caused arithmetic error: Floating divide by 0
% Program caused arithmetic error: Floating illegal operand
IDL> print, min(ee), max(ee)
         -Inf      6.89669

@alaingdl
Copy link
Contributor Author
alaingdl commented Aug 6, 2022

SORRY for the very dramatic way of my message, I was in a middle of processing of some urgent data from JWST, tired, ... And I did not take time to check whether it was a IDL or a GDL problem !!! (furthermore I install a new OS and loss my local old IDL versions)

This is a serious concern in IDL :

on the cluster here, default is IDL 8.5 :

IDL> !version.release
8.5
IDL> rr=findgen(1000)-10 
IDL> ee=sqrt(rr)
% Program caused arithmetic error: Floating illegal operand
IDL> print, min(ee), max(ee)
         -NaN         -NaN
IDL> ee=alog(rr)
% Program caused arithmetic error: Floating divide by 0
% Program caused arithmetic error: Floating illegal operand
IDL> print, min(ee), max(ee)
          NaN          NaN

The trick is here : https://www.l3harrisgeospatial.com/docs/max.html
version 8.5.1 | Always treat NaN's as missing values.

(what a stupid idea to change such an important behavior ! 😢)

What to do ? (I see serious side effect in larges codes ...)

@alaingdl alaingdl changed the title important numerical issue with (at least) SQRT() & ALOG() for negative inputs important numerical issue with (at least) SQRT() & ALOG() for negative inputs (du to change in IDL 8.5.1 in NaN for MIN/MAX) Aug 6, 2022
@alaingdl alaingdl changed the title important numerical issue with (at least) SQRT() & ALOG() for negative inputs (du to change in IDL 8.5.1 in NaN for MIN/MAX) important numerical issue with (at least) SQRT() & ALOG() for negative inputs (due to change in IDL 8.5.1 in NaN for MIN/MAX) Aug 6, 2022
@GillesDuvert
Copy link
Contributor

Actually IMHO (and, should I say, in my educated opinion) NaNs ARE missing values, so it is ample time that IDL treats them as such. See how missing values are tagged in FITS.

Menanig that IDL did take the good decision.

Not that other equivalent software do better, btw. Most choke (or chocked, last time I tried them) on NaNs in a way or another.

I personnaly would love to see GDL ignore all the /NAN options IDL added along the ages and just treat every NAN as 'missing', not just when the NAN option is present (and that would greatly simplify the code). Anybody wants to comment on that?

In the present case, I suppose looking a documentation differences between 8.4 (the max version I have acces to) and current version will show the places where NaN should be taken into account without /NAN, I would argue that the relevant code is already present in GDL, just not triggered.

@brandy125
Copy link
brandy125 commented Aug 6, 2022 via email

@GillesDuvert
Copy link
Contributor

note that GDL has option --fakerelease X.y ( pretend that !VERSION.RELEASE is X.y)
This is just to fool procedures that expect specifics on some version (usually old procedures that would easily be modernized)
But, we could probably adapt GDL's behavior using this.
OTHO, pretending that /NAN is present in every command that has it is very very very easy, just one-liners.
And, this would be for the best for everybody.

@jtappin
Copy link
jtappin commented Aug 8, 2022

A few meandering thoughts here.
Why was it initially done this way? Probably performance, back in the IDL 2.x days computers weren't that fast so not checking for invalid inputs unless told to do so would be a significant gain.

Also while it is likely true that most of the time we do want min(array,/nan) in the absence of a nice Fortran-like ANY or ALL function checking the finiteness of such results is a useful validity check on datasets or simulation outputs. Therefore if min, max etc. don't find NaN, maybe a nan=0 or /nonan is needed.

Also what happens with things like mean in current IDL versions, there I would say giving a finite answer could be dangerous?

@GillesDuvert GillesDuvert self-assigned this Oct 20, 2022
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

4 participants