-
Notifications
You must be signed in to change notification settings - Fork 12
Added Benchmark for Vector #126
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
Conversation
smarr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, though, I got a few thoughts still 😄
Signed-off-by: Stefan Marr <[email protected]>
|
@rhys-h-walker The new Knapsack benchmark fails on SOM++. It segfaults: https://git.cs.kent.ac.uk/sm951/SOMpp/-/jobs/89643#L561 It only segfaults with clang as compiler, COPYING GC, and -DUSE_TAGGING=false -DCACHE_INTEGER=false. Might you have a chance to run this in a debugger to see whether we can figure out what might be wrong on SOM++? I was also wondering whether this here could be described with a loop instead of hardcoding it: |
|
Hi @josrepo we are in the process of integrating the benchmark you added. Do you happen to remember where you got that one from? Would be great, if we could name the original source of it. Thanks! |
|
Hi @smarr, Yes I remember! It's from this Baeldung article: I'll paste it here for completeness: int knapsackDP(int[] w, int[] v, int n, int W) {
if (n <= 0 || W <= 0) {
return 0;
}
int[][] m = new int[n + 1][W + 1];
for (int j = 0; j <= W; j++) {
m[0][j] = 0;
}
for (int i = 1; i <= n; i++) {
for (int j = 1; j <= W; j++) {
if (w[i - 1] > j) {
m[i][j] = m[i - 1][j];
} else {
m[i][j] = Math.max(
m[i - 1][j],
m[i - 1][j - w[i - 1]] + v[i - 1]);
}
}
}
return m[n][W];
}Best, |
|
@josrepo oh, thanks a lot for the fast reply! Do you also happen to remember where you got the item data from? Or whether that's somewhat random. Thanks! |
|
@smarr I'm pretty sure I just made it up! and ran it through the original Java version to determine what the result should be |
|
Perfect. Thanks! |
This works mostly. It also allows us to size the benchmark now, which is useful. I would expect this is still maintaining the focus on Vector operations of the benchmark, but have not checked that to be sure. Signed-off-by: Stefan Marr <[email protected]>
|
@rhys-h-walker I forced pushed to this branch my cleanups. Could you please have a look at my version of Knapsack, whether you see anything obviously problematic? Thanks! |
Looks good to me, quite a bit tidier than my solution :) |
This PR adds two benchmarks:
#,,#remove:, and#at:put:operations#at:, #at:put:,#append:`.The PR also adds fallback to the SOM-st repos on GitHub actions when neither the user nor the pull request target have a fork of the SOM implementation.
Finally, we added two tests to
VectorTest:testAsArrayEnsureItCopiesStorageArraytestDoIndexesDoubleRemoveCore-Lib Updates