Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix comparison comment in snowflake algorithms #3256

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

yacovm
Copy link
Contributor

@yacovm yacovm commented Aug 1, 2024

Why this should be merged

In the snowflake algorithms, there is the following comment:

// terminationConditions[i].alphaConfidence < terminationConditions[i+1].alphaConfidence 
// terminationConditions[i].beta <= terminationConditions[i+1].beta

However, the two lines contradict, as if alpha confidence grows with i, then beta should decrease and not increase. The reason is that the higher our confidence, the less consecutive polls we need to finalize.

How this works

Just a comment fix

How this was tested

No need to test, it's a comment fix.

@yacovm yacovm requested a review from StephenButtolph as a code owner August 1, 2024 14:41
@yacovm yacovm changed the title Fix comparison comment in comment in snowflake algorithms Fix comparison comment in snowflake algorithms Aug 1, 2024
In the snowflake algorithms, there is the following comment:

// terminationConditions[i].alphaConfidence < terminationConditions[i+1].alphaConfidence
// terminationConditions[i].beta <= terminationConditions[i+1].beta

However, the two lines contradict, as if alpha confidence grows with i, then beta should decrease and not increase.
The reason is that the higher our confidence, the less consecutive polls we need to finalize.

Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
@yacovm yacovm force-pushed the fixTypoSnowflake branch from 386dd30 to 32f03f9 Compare August 1, 2024 14:42
@StephenButtolph StephenButtolph added documentation Improvements or additions to documentation or examples cleanup Code quality improvement labels Aug 1, 2024
@StephenButtolph StephenButtolph added this to the v1.11.11 milestone Aug 1, 2024
Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

LGTM - nice catch

@StephenButtolph StephenButtolph added this pull request to the merge queue Aug 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 1, 2024
@StephenButtolph
Copy link
Contributor

🤔 I wonder if we broke the merge queue for PRs coming from forks

@StephenButtolph StephenButtolph added this pull request to the merge queue Aug 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 1, 2024
@StephenButtolph StephenButtolph merged commit 9c3f4a3 into ava-labs:master Aug 1, 2024
20 checks passed
@yacovm
Copy link
Contributor Author

yacovm commented Aug 1, 2024

🤔 I wonder if we broke the merge queue for PRs coming from forks

it's not my first PR from a fork, maybe there could be another explanation?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cleanup Code quality improvement documentation Improvements or additions to documentation or examples
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants