Skip to content

CDFs for EB announcements - #6639

Open
coot wants to merge 2 commits into
leios-prototypefrom
coot/leios-cdfs
Open

CDFs for EB announcements#6639
coot wants to merge 2 commits into
leios-prototypefrom
coot/leios-cdfs

Conversation

@coot

@coot coot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Description

  • CDF counters - refactorisation
  • CDFs for EB announcements

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. These may include:
    • golden tests
    • property tests
    • roundtrip tests
    • integration tests
      See Running tests for more details
  • Any changes are noted in the CHANGELOG.md for affected package
    • cardano-node-chairman, cardano-submit-api and cardano-testnet instead need a
      changelog fragment in <package>/.changes/, because their CHANGELOG.md is generated
      from fragments at release time. Copy _TEMPLATE.yml from that directory, or run
      nix run github:input-output-hk/cardano-dev#herald -- new
  • The version bounds in .cabal files are updated
  • CI passes. See note on CI. The following CI checks are required:
    • Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
    • Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
    • Code builds on Linux, MacOS and Windows for ghc-9.6 and ghc-9.12
  • Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

@coot
coot requested a review from a team as a code owner August 5, 2026 09:51
@coot coot linked an issue Aug 5, 2026 that may be closed by this pull request
@coot coot added the leios Means that this tickets is related to the implementation of ouroboros leios. label Aug 5, 2026
@coot
coot force-pushed the coot/leios-cdfs branch from 1f7b0f3 to ee7ea74 Compare August 5, 2026 12:59

data Config = Config { numOfDataPoints :: Int }

-- | Default `Config` keeps `k/2` data points.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proportionality to k is easier to justify for some CDFs than for others, but it's probably fine for most?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But one other option is to just force each callers to choose a number

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how it's done, we just happen to provide a defaultConfig which is used by all applications (block-fetch and leios announcements). Do you have a better value for leios announcment?

Comment thread cardano-node/src/Cardano/Node/Tracing/Cdf.hs Outdated
Comment thread cardano-node/src/Cardano/Node/Tracing/Cdf.hs Outdated
Comment thread cardano-node/src/Cardano/Node/Tracing/Cdf.hs Outdated
Comment thread cardano-node/src/Cardano/Node/Tracing/Cdf.hs
Comment thread cardano-node/src/Cardano/Node/Tracing/Tracers/Consensus.hs Outdated
Comment thread cardano-node/src/Cardano/Node/Tracing/Tracers/Consensus.hs Outdated
Comment thread cardano-node/src/Cardano/Node/Tracing/Tracers/Consensus.hs
, ("leios.eb.announcement.delay.cdf1600ms", "probability for EB announcemement to complete within 1600ms")
, ("leios.eb.announcement.delay.cdf1800ms", "probability for EB announcemement to complete within 1800ms")
, ("leios.eb.announcement.delay.cdf2000ms", "probability for EB announcemement to complete within 2000ms")
, ("leios.e.b.announcement.late.all", "number of late EB announcements that took longer than 2s: all peers are included")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't expecting "all peers are included" here. These metrics are driven by TraceLeiosAnnouncementAccepted, which is deduped---it only fires the first time some announcement arrives.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all peers are included

What is this supposed to mean? I still don't understand

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there's a typo in the name: .e.b.

Comment thread cardano-node/src/Cardano/Node/Tracing/Tracers/Consensus.hs
@ch1bo
ch1bo force-pushed the leios-prototype branch 9 times, most recently from 2c5ed69 to 22d7144 Compare August 10, 2026 06:06
coot added 2 commits August 17, 2026 13:59
* Moved to a new module
* Provide re-usable API

This is a preparation work for re-using the CDF calculations for Leios.
@coot
coot force-pushed the coot/leios-cdfs branch from ee7ea74 to f3e1719 Compare August 17, 2026 12:20

@nfrisby nfrisby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some "typo"-level minor issues in this thread https://github.com/IntersectMBO/cardano-node/pull/6639/changes#r3722072409 but otherwise looks good to me. Thanks for addressing my comments 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

leios Means that this tickets is related to the implementation of ouroboros leios.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Measure diffusion distributions

2 participants