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

[BUG] - Integers display in scientific notation in SimpleView #2272

Open
rphair opened this issue Jan 10, 2021 · 7 comments
Open

[BUG] - Integers display in scientific notation in SimpleView #2272

rphair opened this issue Jan 10, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@rphair
Copy link

rphair commented Jan 10, 2021

Internal/External External
Area Monitoring
Summary
When using SimpleView, with node 1.24.2 the block producer node began displaying slot numbers for each leadership check in scientific notation. Since these values are either integers or discrete values by nature, showing them as floating point numbers is a senseless complication for programs that process logfiles as well as the human reader.

Steps to reproduce

  1. Put a block producer node in SimpleView via its config.json settings.
  2. Substituting your logfile for node.log if necessary, run (assuming log file begins with 1.24.2 log output):
$ grep 'fromList .("credentials",String "Cardano")' node.log | head -1
  1. See the integer slot number display in scientific notation:
    [core-nyc:cardano.node.Forge:Info:53] [2020-12-09 18:45:14.17 UTC] fromList [("credentials",String "Cardano"),("val",Object (fromList [("kind",String "TraceNodeNotLeader"),("slot",Number 1.5973223e7)]))]

Expected behaviour
Display whole numbers as whole numbers, for compatibility with explorers, concisely written scripts, human readers using grep for quick analysis (without a painstaking conversion to scientific notation), etc.

Human readers trying to make sense of scrolling log files, or write scripts to look up block successes & failures, will wonder why the "New tip" message is displayed as an integer and the leader check is using scientific notation for the same data... which creates difficulties searching back & forth between these two contexts:

$ grep 'Chain extended, new tip:' node.log | head -1

[core-nyc:cardano.node.ChainDB:Notice:43] [2020-12-09 18:45:14.19 UTC] Chain extended, new tip: 47e734df76f1e1c5af3c08cdce761bb7658e624d840f34c8c979493b1ed74c3b at slot 15973213

System info

  • OS: (irrelevant) Ubuntu 20.04
  • Node version 1.24.2 and perhaps beyond

Additional context
This is not the only context where a value is displayed as floating point when its presentation as a whole number or fixed precision decimal would make much more sense. For instance chainDensity, a number always slightly less than 0.05, also is displayed only in scientific notation.

I would therefore ask the developers that are dumping all such diagnostic arrays to please consider the human reader and monitoring script writer in presenting this analytical data in whatever form is most easily read & post-processed.

@rphair rphair added the bug Something isn't working label Jan 10, 2021
@rphair
Copy link
Author

rphair commented Jan 10, 2021

FYI, some attention to this on the Cardano Forum (so far just from me):
https://forum.cardano.org/t/slot-battles-what-do-they-look-like-what-can-be-done-about-them/41316/8

@Jimbo4350
Copy link
Contributor

Closing this. If this is still relevant please re-open.

@rphair
Copy link
Author

rphair commented May 29, 2024

@Jimbo4350 who knows what would have made you close this, since the original report was observable by anyone who cared for a long time afterward. But FYI someone finally noticed the problem & fixed it, as can be seen in a current text log file message from node version 8.9.3:

[core:cardano.node.LeadershipCheck:Info:304] [2024-05-29 16:40:51.00 UTC] {"chainDensity":4.7917083e-2,"credentials":"Cardano","delegMapSize":1344990,"kind":"TraceStartLeadershipCheck","slot":125434560,"utxoSize":11175231}

i.e. a slot number without the crippling scientific notation (after waiting over 2 years for this to appear as an integer):

"slot":125434560,

@rphair
Copy link
Author

rphair commented Sep 7, 2024

@Jimbo4350 it's not only strange that this was closed with no explanation but also that I can't reopen it. If this problem is something the node maintainers don't want to fix, they need to say here why they're not fixing it.

Here's a line from the 9.1.1 node log in SimpleView, highlighting the integers that are appearing in scientific notation:

[core:cardano.node.Forge:Info:404] [2024-09-07 14:29:51.05 UTC] fromList [("credentials",String "Cardano"),("val",Object (fromList [("block",String "729b2ce73ed61bbd75d145d43cdb6e0d1de75b34bdb4110df35389b991c51765"),("blockNo",Number 1.0805114e7),("blockPrev",String "8c6ca9d57ed67f16b87592ad68249755e7f52f7ca5afd70fb1e54ee99b5fccd0"),("kind",String "TraceForgedBlock"),("slot",Number 1.341531e8)]))]

This is the crucial part: the slot number (an integer) doesn't appear in enough precision to tell what the original integer was. How is this logging therefore to be of any use for debugging?

I'd been waiting for this to be fixed in the 9.x overhaul but now assume by default the developers still think the logs don't need to be readable either by humans or scripts. Rather than wait for 10.x I believe this should be fixable any time with a small change to the data types in the routine that is dumping this data structure into the logfile.

Until we get some acknowledgement I would have to keep tagging people that seem responsive about these things, at least until this is reopened & given a proper triage & response please - cc 🙏 @disassembler @erikd @karknu

@carbolymer carbolymer reopened this Sep 9, 2024
@rphair rphair changed the title [BUG] - Integers display in scientific notation in SimpleView (e.g. leader check slot numbers) [BUG] - Integers display in scientific notation in SimpleView Sep 9, 2024
@carbolymer
Copy link
Contributor

@rphair Is that still the case for integers like blockNo or slotNo? From what I saw, there should be normal integers, without scientific notation. However we still have chain density as scientific notation.

This can be fixed by using custom Value -> ByteString encoder for JSON:

import qualified Data.Aeson as Aeson
import qualified Data.Aeson.Encoding as Aeson
import qualified Data.Aeson.KeyMap as Aeson
import qualified Data.Aeson.Key as Aeson
import qualified Data.Scientific as S
import qualified Data.ByteString.Builder.Scientific as S
import qualified Data.ByteString.Builder as BS
import qualified Data.ByteString.Lazy as BSL


encode' :: ToJSON a => a -> BSL.ByteString
encode' = Aeson.encodingToLazyByteString . Aeson.unsafeToEncoding . encodeToBuilder . toJSON

encodeToBuilder :: Value -> BS.Builder
encodeToBuilder = \case
  (Number n) -> scientific n
  (Array v) -> array v
  (Object m) -> object m
  nonNumber -> toAesonBuilder nonNumber
  where
    -- use native aeson encoding for non-numbers
    toAesonBuilder :: ToJSON v => v -> BS.Builder
    toAesonBuilder = Aeson.fromEncoding . Aeson.toEncoding

    array :: V.Vector Value -> BS.Builder
    array v
      | V.null v  = Aeson.fromEncoding $ Aeson.toEncoding v
      | otherwise = BS.char8 '[' <>
                    encodeToBuilder (V.unsafeHead v) <>
                    V.foldr withComma (BS.char8 ']') (V.unsafeTail v)
      where
        withComma a z = BS.char8 ',' <> encodeToBuilder a <> z

    object :: Aeson.KeyMap Value -> BS.Builder
    object m = case Aeson.toList m of
        (x:xs) -> BS.char8 '{' <> one x <> foldr withComma (BS.char8 '}') xs
        []     -> toAesonBuilder (mempty :: Aeson.KeyMap Value)
      where
        withComma a z = BS.char8 ',' <> one a <> z
        one (k,v)     = key k <> BS.char8 ':' <> encodeToBuilder v
        key = toAesonBuilder . Aeson.String . Aeson.toText

    scientific v 
      | v < 1 = S.formatScientificBuilder S.Fixed Nothing v
      | otherwise = S.formatScientificBuilder S.Fixed (Just 0) v


-- >>> encode' $ Aeson.object ["slot" .= (0.00000003 :: Float) ]
-- "{\"slot\":0.00000003}"
--
-- >>> Aeson.encode $ Aeson.object ["slot" .= (0.00000003 :: Float) ]
-- "{\"slot\":3.0e-8}"

-- >>> encode' $ Aeson.object ["slot" .= (maxBound :: Word64) ]
-- "{\"slot\":18446744073709551615}"

-- >>> Aeson.encode $ Aeson.object ["slot" .= (maxBound :: Word64) ]
-- "{\"slot\":18446744073709551615}"

TBD: where this encoder should be placed

@rphair
Copy link
Author

rphair commented Sep 27, 2024

thanks @carbolymer - as of node 9.2.1 with "ViewMode": "SimpleView" set in config.json, the slot number comes as integer or scientific notation depending on log facility:

  • [core:cardano.node.LeadershipCheck:Info:262] [2024-09-27 15:41:22.00 UTC] {"chainDensity":4.9134064e-2,"credentials":"Cardano","delegMapSize":1335715,"kind":"TraceStartLeadershipCheck","slot":135885391,"utxoSize":11264350}
  • [core:cardano.node.Forge:Info:262] [2024-09-27 15:41:22.04 UTC] fromList [("credentials",String "Cardano"),("val",Object (fromList [("kind",String "TraceNodeNotLeader"),("slot",Number 1.35885391e8)]))]

Block number is also in scientific notation when logging successfully forged blocks, as well as the slot number (same as reported above already):

  • [core:cardano.node.Forge:Info:262] [2024-09-27 15:17:54.04 UTC] fromList [("credentials",String "Cardano"),("val",Object (fromList [("kind",String "TraceNodeIsLeader"),("slot",Number 1.35883983e8)]))]
  • [core:cardano.node.Forge:Info:262] [2024-09-27 15:17:54.04 UTC] fromList [("credentials",String "Cardano"),("val",Object (fromList [("block",String "36332156c2440baa90dfc16bebb66968ec80962ce2497759fd76e9b4d2b66ffe"),("blockNo",Number 1.0889665e7),("blockPrev",String "7c016df17d379770e5bbb17161f4e3a85f4af5a1dcdcd55b5b2cc37f93f57185"),("kind",String "TraceForgedBlock"),("slot",Number 1.35883983e8)]))]

I do think it would be more readable and parseable to have chainDensity as an unexponented decimal, rather than scientific notation: especially since it will not be changing magnitude.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants