Is this N/3 correct? #1886
Comments
|
And if it's changed to |
|
Yeah I think I agree here. Fortunately this is just a test for correct
compilation and not a benchmark.
…On Sun, May 19, 2024 at 11:00 AM Alec Jacobson ***@***.***> wrote:
https://github.com/EnzymeAD/Enzyme/blob/2188ad8f1401886eca1e2045abad6b60424fc2c5/enzyme/test/Integration/Sparse/ringspring3Drestlengthone.cpp#L24
N seems to be the number of 3D nodes. So that pos and x are arrays of
length 3*N.
This N/3 seems to mean that only first ⅓ of the springs are considered.
—
Reply to this email directly, view it on GitHub
<#1886>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJTUXHRZ74TX2H6742GI3DZDDEBRAVCNFSM6AAAAABH6PENKGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGMYDINRXGQ3DKNI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
I'm not sure about the % . I need to double check what this test is validating for, but the wrap around in the actual benchmarks is done via the todense to create the actual double pointers (and accessing them will implicitly wrap around) |
|
Yeah okay I think this is only a partial test here
The commented out line
// input = __enzyme_todense((void*)mod_load, (void*)never_store, input, N);
is an example of what would be needed to ensure that the wrap around
happens internally if one doesn’t do so in the inner function.
@martinjm97 this is the actual benchmark code here right?
https://github.com/martinjm97/sparse_deriv_enzyme/blob/main/rest_length_1/enzyme_dense_hess.cpp
Or at least it does have the proper sizing and inner modulus you’re looking
for here.
We for sure should just add the full benchmark as another integration test
though
…On Sun, May 19, 2024 at 11:03 AM William Moses ***@***.***> wrote:
Yeah I think I agree here. Fortunately this is just a test for correct
compilation and not a benchmark.
On Sun, May 19, 2024 at 11:00 AM Alec Jacobson ***@***.***>
wrote:
>
> https://github.com/EnzymeAD/Enzyme/blob/2188ad8f1401886eca1e2045abad6b60424fc2c5/enzyme/test/Integration/Sparse/ringspring3Drestlengthone.cpp#L24
>
> N seems to be the number of 3D nodes. So that pos and x are arrays of
> length 3*N.
>
> This N/3 seems to mean that only first ⅓ of the springs are considered.
>
> —
> Reply to this email directly, view it on GitHub
> <#1886>, or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJTUXHRZ74TX2H6742GI3DZDDEBRAVCNFSM6AAAAABH6PENKGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGMYDINRXGQ3DKNI>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
|
|
Thanks. @martinjm97 do you mind adding me as a collaborator on that repo so I can copy the benchmark for our comparison? |
|
Sorry, I just saw this. I've added you to the repo. |
|
The benchmarking script is here: The code that we run is in the test is here: |
|
I don't think I have access to that repo, @deomaiidae if you can add me |
|
@alecjacobson the code has a bug as you pointed out. Moreover, the benchmarking script calls the file at this location. However, when I ran the benchmarking script, I ran a version of the code that fixed this bug (and also printed out how much time the benchmark took to run). I've made a PR of the code I ran: #1889. |
|
Yeah that post sparse todense is the one I remember working with you on during benchmarking, so that looks right. |
Enzyme/enzyme/test/Integration/Sparse/ringspring3Drestlengthone.cpp
Line 24 in 2188ad8
Nseems to be the number of 3D nodes. So thatposandxare arrays of length3*N.This
N/3seems to mean that only first ⅓ of the springs are considered.The text was updated successfully, but these errors were encountered: