Automatic exponential formatting in Debug #86479

ExpHP · 2021-06-20T01:06:58Z

Context: See this comment from the libs team

Makes "{:?}" switch to exponential for floats based on magnitude. The libs team suggested exploring this idea in the discussion thread for RFC rust-lang/rfcs#2729. (note: this is not an implementation of the RFC; it is an implementation of one of the alternatives)

Thresholds chosen were 1e-4 and 1e16. Justification described here.

This will require a crater run.

As mentioned in the commit message of 8731d4d, this behavior will not apply when a precision is supplied, because I wanted to preserve the following existing and useful behavior of {:.PREC?} (which recursively applies {:.PREC} to floats in a struct):

assert_eq!(
    format!("{:.2?}", [100.0, 0.000004]),
    "[100.00, 0.00]",
)

I looked around and am not sure where there are any tests that actually use this in the test suite, though?

All things considered, I'm surprised that this change did not seem to break even a single existing test in x.py test --stage 2. (even when I tried a smaller threshold of 1e6)

* {:.PREC?} already had legitimately useful behavior (recursive formatting of structs using fixed precision for floats) and I suspect that changes to the output there would be unwelcome. (besides, precision introduces sinister edge cases where a number can be rounded up to one of the thresholds) Thus, the new behavior of Debug is, "dynamically switch to exponential, but only if there's no precision." * This could not be implemented in terms of float_to_decimal_common without repeating the branch on precision, so 'float_to_general_debug' is a new function. The name is '_debug' instead of '_common' because the considerations in the previous bullet make this logic pretty specific to Debug. * 'float_to_decimal_common' is now only used by Display, so I inlined the min_precision argument and renamed the function accordingly.

rust-highfive · 2021-06-20T01:07:01Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

Mark-Simulacrum · 2021-06-28T14:56:25Z

r? @yaahc since you left rust-lang/rfcs#2729 (comment)

yaahc · 2021-06-28T23:34:10Z

This is a breaking change. (obviously)

I'm not sure it's accurate to call this a breaking change, or I at least worry it will be misleading. We explicitly don't consider Debug output to be stable: https://doc.rust-lang.org/stable/std/fmt/trait.Debug.html#stability

I looked around and am not sure where there are any tests that actually use this in the test suite, though?

If you wanted to go ahead and add some tests for the manual precision case that would be extremely appreciated though it is not required.

crlf0710 · 2021-07-17T03:13:47Z

Triage: What's next steps here?

yaahc · 2021-07-28T22:06:36Z

I think next steps are to run a crater experiment.

@bors try

bors · 2021-07-28T22:06:47Z

⌛ Trying commit 8731d4d with merge 9bcd538c2638edd572fdfcbb0adddf2b83c494c3...

bors · 2021-07-28T22:10:55Z

💔 Test failed - checks-actions

rust-log-analyzer · 2021-07-28T22:11:31Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Found online and idle hosted runner in the current repository's organization account that matches the required labels: 'ubuntu-latest-xl'
Waiting for a Hosted runner in the 'organization' to pick this job...
Job is waiting for a hosted runner to come online.
Job is about to start running on the hosted runner: Hosted Agent (hosted)
##[group]Operating System
Ubuntu
18.04.5
LTS
---
From https://github.com/rust-lang/miri
 * branch              062cf07d98822beb4c5b1132f47b4397275f403a -> FETCH_HEAD
 * branch              06758c48bd7a77bb5aa43fc50cf344540ba5afef -> FETCH_HEAD
 * branch              dd7f545a69e4b720407e458bf4ade0b207bbf9ee -> FETCH_HEAD
fatal: remote error: upload-pack: not our ref 9ad6e5b8f68ee4bcd85886e50b8b0a70cbb91a52
Errors during submodule fetch:
##[error]Process completed with exit code 1.
Post job cleanup.

yaahc · 2021-07-28T22:16:10Z

Looks like some sort of git error, not sure what's up with that... I guess let's see if that was a fluke.

@bors retry

bors · 2021-07-28T22:16:23Z

⌛ Trying commit 8731d4d with merge d2e9fff28e244f1ea4927194f1f9b77f93e2ffe2...

bors · 2021-07-28T23:04:00Z

☀️ Try build successful - checks-actions
Build commit: d2e9fff28e244f1ea4927194f1f9b77f93e2ffe2 (d2e9fff28e244f1ea4927194f1f9b77f93e2ffe2)

yaahc · 2021-07-28T23:04:43Z

@craterbot build-and-test

craterbot · 2021-07-28T23:04:44Z

🚨 Error: failed to parse the command

🆘 If you have any trouble with Crater please ping @rust-lang/infra!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

yaahc · 2021-07-28T23:05:33Z

@craterbot run

craterbot · 2021-07-28T23:05:39Z

👌 Experiment pr-86479 created and queued.
🤖 Automatically detected try build d2e9fff28e244f1ea4927194f1f9b77f93e2ffe2
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2021-08-02T03:12:48Z

🚧 Experiment pr-86479 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2021-08-07T18:57:32Z

🎉 Experiment pr-86479 is completed!
📊 70 regressed and 54 fixed (175230 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

yaahc · 2021-10-07T20:40:59Z

I reviewed the results of the crater run and it seems like only 2 or 3 of the test failures were due to the float formatting change.

The rest seem to all be spurious failures. This seems like an acceptable level of breakage to me.

I'm going to start an FCP since this does change runtime behavior, though this is category of runtime breakage that we explicitly reserve the right to make, so I'm going to treat this as a @rust-lang/libs issue rather than a libs-api issue.

@rfcbot merge

rfcbot · 2021-10-07T20:41:00Z

Team member @yaahc has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

ExpHP · 2021-10-07T21:49:51Z

Perhaps the relevant code authors should be tagged so they can update the affected projects?

@rkuhn for the cbor-data regression
@0b11001111 for the is-close regression
@cpmech for the russel_chk regression

For this small sample of three results it surprises me how different they are:

cbor-data is using Debug in a Display impl, which I would consider to be a legitimate stability bug in the crate; it would be better to use e.g. the dtoa crate, or to manually add the .0 suffix if non-exponential format is desirable. (it is once again a shame that such is not the behavior of {:#}...)
is-close's regression is purely a test suite regression; the suite contains a direct and deliberate test of Debug output for a struct, so this is definitely expected breakage.
russel_chk's assert_approx_eq macro seems like a perfectly reasonable use of {:?}, but unfortunately the test suite contains a #[should_panic] message which breaks as the 1e-5 tolerance is now printed in exponential. Perhaps this test will want to use a larger tolerance in the meanwhile so it can succeed on both current and future Rust.

cpmech · 2021-10-07T22:20:03Z

@ExpHP thanks for letting me know! My code is now fixed: https://github.com/cpmech/russell/pull/32/commits
Cheers ;-)

joshtriplett · 2021-10-08T06:47:38Z

Not to nitpick, but I personally think a threshold of 1e-6 might be more readable more often.

Either way, though:
@rfcbot reviewed

ExpHP · 2021-10-08T16:16:17Z

I don't disagree. The choice of this large threshold was partly motivated by the similarly large thresholds in languages like Javascript and Python where this formatting is done automatically.

That said, it occurs to me now that python has implicit int -> float conversion and Javascript has no native integer type. These facts probably help to motivate such large thresholds in those languages, and as such could serve as arguments for a smaller threshold in rust. I am unsure.

joshtriplett · 2021-10-08T19:41:08Z

To clarify, I think 1e16 is a fine upper threshold. I'm suggesting that the lower threshold should be lowered to 1e-6, allowing (for instance) 0.00003 to display without switching to exponential notation.

ExpHP · 2021-10-08T20:58:05Z

Ah. I misread. There is precedence for 1e-6 in Javascript, but I think 1e-4 is more common.

I guess this boils down to personal preference. When I see 0.0001 I see "a tenth of a milli-unit" or "1% of 1%". When I see 0.00001 I feel like I need to count the zeroes.

rfcbot · 2021-10-08T21:19:46Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot · 2021-10-18T21:20:53Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

rkuhn · 2021-10-19T14:35:35Z

Thanks @ExpHP for the ping, I went with the regular Display impl because the trailing .0 is not essential for me — the next release will contain breaking changes in any case.

yaahc · 2021-10-19T18:10:39Z

@bors r+

bors · 2021-10-19T18:10:40Z

📌 Commit 8731d4d has been approved by yaahc

Rollup of 10 pull requests Successful merges: - rust-lang#86479 (Automatic exponential formatting in Debug) - rust-lang#87404 (Add support for artifact size profiling) - rust-lang#87769 (Alloc features cleanup) - rust-lang#88789 (remove unnecessary bound on Zip specialization impl) - rust-lang#88860 (Deduplicate panic_fmt) - rust-lang#90009 (Make more `From` impls `const` (libcore)) - rust-lang#90018 (Fix rustdoc UI for very long type names) - rust-lang#90025 (Revert rust-lang#86011 to fix an incorrect bound check) - rust-lang#90036 (Remove border-bottom from most docblocks.) - rust-lang#90060 (Update RELEASES.md) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned Mark-Simulacrum Jun 20, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2021

rust-highfive assigned yaahc and unassigned Mark-Simulacrum Jun 28, 2021

Mark-Simulacrum added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 28, 2021

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2021

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 28, 2021

craterbot removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Aug 7, 2021

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 7, 2021

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 8, 2021

rkuhn mentioned this pull request Oct 15, 2021

harden string representation for numbers Actyx/cbor-data#14

Closed

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 19, 2021

ExpHP mentioned this pull request Oct 19, 2021

General floating point formatting in Debug with {:g?} rust-lang/rfcs#2729

Closed

JohnTitor mentioned this pull request Oct 19, 2021

Rollup of 10 pull requests #90067

Merged

bors merged commit ca6798a into rust-lang:master Oct 20, 2021

rustbot added this to the 1.58.0 milestone Oct 20, 2021

apiraino removed the to-announce Announce this issue on triage meeting label Oct 21, 2021

alamb mentioned this pull request Jan 14, 2022

Update test output for Rust 1.58 release apache/arrow-rs#1173

Closed

Skgland mentioned this pull request Feb 2, 2022

thread 'main' panicked at 'index out of bounds when trying to load Logtalk mthom/scryer-prolog#1233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic exponential formatting in Debug #86479

Automatic exponential formatting in Debug #86479

ExpHP commented Jun 20, 2021 •

edited

Loading

rust-highfive commented Jun 20, 2021

Mark-Simulacrum commented Jun 28, 2021

yaahc commented Jun 28, 2021 •

edited

Loading

crlf0710 commented Jul 17, 2021

yaahc commented Jul 28, 2021

bors commented Jul 28, 2021

bors commented Jul 28, 2021

rust-log-analyzer commented Jul 28, 2021

yaahc commented Jul 28, 2021

bors commented Jul 28, 2021

bors commented Jul 28, 2021

yaahc commented Jul 28, 2021

craterbot commented Jul 28, 2021

yaahc commented Jul 28, 2021

craterbot commented Jul 28, 2021

craterbot commented Aug 2, 2021

craterbot commented Aug 7, 2021

yaahc commented Oct 7, 2021 •

edited

Loading

rfcbot commented Oct 7, 2021 •

edited by cuviper

Loading

ExpHP commented Oct 7, 2021 •

edited

Loading

cpmech commented Oct 7, 2021

joshtriplett commented Oct 8, 2021

ExpHP commented Oct 8, 2021

joshtriplett commented Oct 8, 2021

ExpHP commented Oct 8, 2021

rfcbot commented Oct 8, 2021

rfcbot commented Oct 18, 2021

rkuhn commented Oct 19, 2021

yaahc commented Oct 19, 2021

bors commented Oct 19, 2021

Automatic exponential formatting in Debug #86479

Automatic exponential formatting in Debug #86479

Conversation

ExpHP commented Jun 20, 2021 • edited Loading

rust-highfive commented Jun 20, 2021

Mark-Simulacrum commented Jun 28, 2021

yaahc commented Jun 28, 2021 • edited Loading

crlf0710 commented Jul 17, 2021

yaahc commented Jul 28, 2021

bors commented Jul 28, 2021

bors commented Jul 28, 2021

rust-log-analyzer commented Jul 28, 2021

yaahc commented Jul 28, 2021

bors commented Jul 28, 2021

bors commented Jul 28, 2021

yaahc commented Jul 28, 2021

craterbot commented Jul 28, 2021

yaahc commented Jul 28, 2021

craterbot commented Jul 28, 2021

craterbot commented Aug 2, 2021

craterbot commented Aug 7, 2021

yaahc commented Oct 7, 2021 • edited Loading

rfcbot commented Oct 7, 2021 • edited by cuviper Loading

ExpHP commented Oct 7, 2021 • edited Loading

cpmech commented Oct 7, 2021

joshtriplett commented Oct 8, 2021

ExpHP commented Oct 8, 2021

joshtriplett commented Oct 8, 2021

ExpHP commented Oct 8, 2021

rfcbot commented Oct 8, 2021

rfcbot commented Oct 18, 2021

rkuhn commented Oct 19, 2021

yaahc commented Oct 19, 2021

bors commented Oct 19, 2021

ExpHP commented Jun 20, 2021 •

edited

Loading

yaahc commented Jun 28, 2021 •

edited

Loading

yaahc commented Oct 7, 2021 •

edited

Loading

rfcbot commented Oct 7, 2021 •

edited by cuviper

Loading

ExpHP commented Oct 7, 2021 •

edited

Loading