-
Notifications
You must be signed in to change notification settings - Fork 130
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
Fix NPS measurement for TC scaling #2081
base: master
Are you sure you want to change the base?
Conversation
Tested with https://tests.stockfishchess.org/tests/view/66729718602682471b065101?show_task=66 Works as expected |
I think that's reasonable direction. Things to consider (idk the right answer)
|
19ccd13
to
3520a59
Compare
@Disservin the workers look good with the PR. As highlighted by @vondele, merging the PR will change the TC for system with high concurrency, with a jump in the PT. |
f10794f
to
3bed7a9
Compare
New commit with: simplification with MNps master vs PR for both normal test (thread=1) and SMP test (threads=8). As expected with master there is not a difference for threads > 1.
|
6f58f79
to
465a270
Compare
I experimented a bit with I have these concerns in using
Here are my tests:
Click to view
Click to view
Click to view
Click to view
Click to view
Click to view
|
Done.
We can run
To be discussed. |
Co-Authored-By: Jamie Whiting <[email protected]>
465a270
to
42fb071
Compare
42fb071
to
d63fcfa
Compare
This PR modifies the NPS measurement for TC scaling to more closely resemble actual testing conditions. In particular, it addresses the point raised in #2077
Instead, this PR runs a bench process for each active core and takes the average NPS.