Skip to content

Avoid floating point false alarms in the TCP model's missing ACK assertion #433

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

Merged
merged 2 commits into from
Jun 27, 2025

Conversation

nfrisby
Copy link
Collaborator

@nfrisby nfrisby commented Jun 26, 2025

Fixes #393.

@nfrisby nfrisby requested a review from bwbush June 26, 2025 17:55
@nfrisby nfrisby self-assigned this Jun 26, 2025
@nfrisby
Copy link
Collaborator Author

nfrisby commented Jun 26, 2025

@bwbush this runs out of memory even on my 32GB machine, so I can't test my theory.

I've pushed a couple commits so you can.

  • First commit skips the "triangle inequality check", which was a slow distraction for this Issue.
  • Second commit rearranges the arithmetic/inequality in case it's a false alarm caused by floating point issues.
  • Second commit also now prints out the offending numbers for future debugging.

@nfrisby nfrisby force-pushed the nfrisby/issue-393-hask-crash branch 2 times, most recently from 7f5e290 to 327ba1c Compare June 26, 2025 18:06
Copy link
Collaborator

@bwbush bwbush left a comment

Choose a reason for hiding this comment

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

This executes correctly at 327ba1c far past the point at which the assertion failed in issue #393, but using the same configuration and network topology.

@nfrisby
Copy link
Collaborator Author

nfrisby commented Jun 27, 2025

Great. I'll tidy it up and merge it today.

@nfrisby nfrisby force-pushed the nfrisby/issue-393-hask-crash branch from 327ba1c to 651584e Compare June 27, 2025 18:05
nfrisby and others added 2 commits June 27, 2025 11:07
The FP arithmetic that leads to the branch with the assertion and the
assertion's FP arithmetic are now in monotonic relationship.
@nfrisby nfrisby marked this pull request as ready for review June 27, 2025 18:07
@nfrisby nfrisby force-pushed the nfrisby/issue-393-hask-crash branch from 651584e to 1a643aa Compare June 27, 2025 18:07
@nfrisby
Copy link
Collaborator Author

nfrisby commented Jun 27, 2025

@bwbush FYI this PR also adds the --skip-triangle-inequality-check to the sim subcommand, since its time complexity is painful with large topologies and it's not particularly critical. (And apparently the real world data doesn't satisfy it.)

@nfrisby nfrisby merged commit 4d9dced into main Jun 27, 2025
5 of 6 checks passed
@nfrisby nfrisby deleted the nfrisby/issue-393-hask-crash branch June 27, 2025 18:30
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

Successfully merging this pull request may close these issues.

Haskell simulator crashes on pseudo-mainnet
2 participants