Skip to content

Conversation

@apoelstra
Copy link
Collaborator

No description provided.

"best_value": 785.51375,
"median_name": "62",
"median_value": 800.2093333333335,
"worst_name": "17",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are some of these names like "17" and "62"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess they have always been like this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think these ones have no name (there is a single unnamed distribution) and the name-parsing code gets confused by it.

Copy link
Contributor

@roconnor-blockstream roconnor-blockstream left a comment

Choose a reason for hiding this comment

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

LGTM.

Probably FeNormalize is the most notable outlier, going from around 289 to 360, but even this is probably acceptable. I don't think there is any reason to believe FeNormalize's benchmarking has changed in any fundamental way.

@apoelstra
Copy link
Collaborator Author

Great, thanks! Yeah, I suspect these low values are just noisy. (Though it does seem like the distributions are all pretty tight.)

It is certainly plausible that the marshalling code has changed since 2024 in ways that would affect these values. When I get a chance I can try to bisect this.

Anyway I think we should consider the new ones to be more authoritative than the old ones.

@roconnor-blockstream
Copy link
Contributor

FeNormalize was 311 in 2025-10, which was much closer to 289.

Copy link
Collaborator Author

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

On 72c3454 successfully ran local tests

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.

2 participants