Conversation
AA-Turner
reviewed
Aug 25, 2023
| a3 = 1.781477937 | ||
| a4 = -1.821255978 | ||
| a5 = 1.330274429 | ||
| pi = 3.141592653589793238462643 |
There was a problem hiding this comment.
Could you just use math.pi here? These constants in general feel a little 'magic', is there a CDF function in the stdlib or numpy that could be used instead?
Contributor
Author
There was a problem hiding this comment.
There is a CDF function in Scipy, with different behavior as to how the wraparound is handled. So it's both too big a dependency for this little thing, IMHO, and it doesn't do what we need anyway.
But, yes, could use math.pi.
AA-Turner
reviewed
Aug 25, 2023
| import functools | ||
| import json | ||
| import os | ||
| from typing import Dict, Optional, Tuple |
There was a problem hiding this comment.
You could use from __future__ import annotations and dispense with these typing imports by using PEP 585 types
Support for the Heirarchical Performance Testing (HPT) method in this paper: T. Chen, Y. Chen, Q. Guo, O. Temam, Y. Wu and W. Hu, "Statistical performance comparisons of computers," IEEE International Symposium on High-Performance Comp Architecture, New Orleans, LA, USA, 2012, pp. 1-12, doi: 10.1109/HPCA.2012.6169043. This is largely a direct port of the bash implementation available here: https://github.com/cirosantilli/parsec-benchmark/tree/master/toolkit/hpt This approach is a more robust way to measure overall effectiveness across a number of benchmarks. It is still biased in that the benchmarks should be a representative sample, but it accounts for the fact that some benchmarks are more reproducible and reliable than others. It has been modified so that each benchmark can have a different number of samples (the original code assumed the matrix was rectangular, but there is nothing about the method itself that should require that).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Support for the Heirarchical Performance Testing (HPT) method in this paper:
T. Chen, Y. Chen, Q. Guo, O. Temam, Y. Wu and W. Hu,
"Statistical performance comparisons of computers,"
IEEE International Symposium on High-Performance Comp Architecture,
New Orleans, LA, USA, 2012, pp. 1-12,
doi: 10.1109/HPCA.2012.6169043.
This is largely a direct port of the bash implementation available here:
https://github.com/cirosantilli/parsec-benchmark/tree/master/toolkit/hpt
This approach is a more robust way to measure overall effectiveness across a number of benchmarks. It is still biased in that the benchmarks should be a representative sample, but it accounts for the fact that some benchmarks are more reproducible and reliable than others.
It has been modified so that each benchmark can have a different number of samples (the original code assumed the matrix was rectangular, but there is nothing about the method itself that should require that).