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

Conversion from string to real does not accept non-finite values #73

Closed
1 of 5 tasks
JohnReppy opened this issue Jul 15, 2022 · 4 comments
Closed
1 of 5 tasks

Conversion from string to real does not accept non-finite values #73

JohnReppy opened this issue Jul 15, 2022 · 4 comments
Assignees
Labels
basis-lib problem with Standard ML Basis library bug Something isn't working fixed-in-110.99.3 issues that will be fixed in the 110.99.3 version floating-point problem related to floating-point operations gforge bug (or feature request) ported from smlnj-gforge repository

Comments

@JohnReppy
Copy link
Contributor

JohnReppy commented Jul 15, 2022

Version

110.99

Operating System

  • All
  • Linux
  • macOS
  • Windows
  • Other Unix

OS Version

No response

Processor

No response

Component

Basis Library

Severity

Minor

Description of the problem

In the Basis Library, in signature REAL, the description of the functions scan and fromString states:

It also accepts the following string representations of non-finite values:
[+~-]?(inf | infinity | nan)
where the alphabetic characters are case-insensitive.

However these functions both return NONE for such non-finite values.
Additional comments:
Copied from polyml/polyml#181

Transcript

No response

Expected Behavior

No response

Steps to Reproduce

The following expressions should match SOME _:

  StringCvt.scanString Real.scan "nan";
  Real.fromString "-inf";

Additional Information

No response

Email address

phil.clayton@veonix.com

Comments from smlnj-gforge

Original smlnj-gforge bug number 317

Submitted via web form by Phil Clayton phil.clayton@veonix.com on 2022-07-05 at 13:17:00

Keywords: reals

comment by @JohnReppy on 2022-07-05 14:37:00 +000 UTC

Fixed for 110.99.3 and 2022.1

@JohnReppy JohnReppy added basis-lib problem with Standard ML Basis library bug Something isn't working floating-point problem related to floating-point operations gforge bug (or feature request) ported from smlnj-gforge repository labels Jul 15, 2022
@JohnReppy JohnReppy self-assigned this Jul 15, 2022
@pclayton
Copy link

Thank you for the update. I have tested the fixed version and have the following observations:

  1. Exception match may be raised:

    - Real.fromString "n";
    
    uncaught exception Match [nonexhaustive match failure]
      raised at: Basis/Implementation/real-scan.sml:106.62
    

    Similarly for "na" and case variants. Note that SML/NJ does warn about this when compiling:

    [compiling $SMLNJ-BASIS/(basis.cm):(basis-common.cm):Implementation/(sources.cm):real-scan.sml]
    Basis/Implementation/real-scan.sml:105.46-106.62 Warning: match nonexhaustive
              SOME cs' => ...
    
  2. Should scan and fromString generate a signed NaN, as fromDecimal does? The Basis Library does not appear to require this. Currently the sign is ignored, so fromString "+nan" and fromString "-nan" have the same sign bit. Is the intention that Option.composePartial (fromDecimal, IEEEReal.fromString) is a possible implementation of fromString but is more constrained (to produce a signed NaN)?

@JohnReppy JohnReppy added the fixed-in-110.99.3 issues that will be fixed in the 110.99.3 version label Jul 19, 2022
@JohnReppy
Copy link
Contributor Author

I'll fix the scanning bug.

NaNs are not signed. At the time that the SML Basis Library was specified, the IEEE FP standard did not specify any meaning for the sign bit. More recently, the bit has been specified to distinguish between signaling and quiet NaNs (1 means quiet). Signaling NaNs are problematic for SML (assuming that we would map them to exceptions), since it is difficult to map them to exceptions in a precise way. For example,

(sNaN + 1.0; x := false)

(where sNaN is a signaling NaN) might do the assignment, before the exception get signaled. So I think that we should just stick with quiet NaNs in all cases (i.e., with the sign bit set).

@pclayton
Copy link

I thought that signaling/quiet bit is the MSB of the mantissa, not the sign bit. Wikipedia states:

the most significant bit of the significand field is exclusively used to distinguish between quiet and signaling NaNs

Therefore the level of abstraction in the Basis Library does not allow the signaling/quiet bit of a NaN to be observed or changed, however the (meaningless) sign bit can be observed and changed.

JohnReppy added a commit that referenced this issue Jul 19, 2022
preserving the sign of NaNs when scanning.
@JohnReppy
Copy link
Contributor Author

You're correct about the quiet bit (I shouldn't be thinking about floating point in the middle of the night).

I've fixed the scanning bug and added support for setting the sign bit. As you noted, the Basis Library specification does not specify how the sign is interpreted when scanning a NaN and it definitely ignores the sign when printing. There should be a proposed clarification added to https://github.com/SMLFamily/BasisLibrary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
basis-lib problem with Standard ML Basis library bug Something isn't working fixed-in-110.99.3 issues that will be fixed in the 110.99.3 version floating-point problem related to floating-point operations gforge bug (or feature request) ported from smlnj-gforge repository
Projects
None yet
Development

No branches or pull requests

2 participants