Is this N/3 correct?

[alecjacobson]

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.

[alecjacobson]

And if it's changed to N then it seems the i*3+4 etc. need a %(N*3) to wrap around the circle of springs.

[wsmoses]

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 https://github.com/EnzymeAD/Enzyme/issues/1886, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJTUXHRZ74TX2H6742GI3DZDDEBRAVCNFSM6AAAAABH6PENKGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGMYDINRXGQ3DKNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[wsmoses]

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)

[wsmoses]

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 https://github.com/EnzymeAD/Enzyme/issues/1886, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJTUXHRZ74TX2H6742GI3DZDDEBRAVCNFSM6AAAAABH6PENKGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGMYDINRXGQ3DKNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[alecjacobson]

Thanks. @martinjm97 do you mind adding me as a collaborator on that repo so I can copy the benchmark for our comparison?

[martinjm97]

Sorry, I just saw this. I've added you to the repo.

[martinjm97]

The benchmarking script is here:

https://github.com/dromaiidae/sparse_deriv/blob/siggraph_benchmarking/benchmark/SiggraphPerformance/ring_spring_rest_length_one_timing.py

The code that we run is in the test is here: https://github.com/dromaiidae/sparse_deriv/blob/dc973f23d672232089c30efe9bccabf48ebab765/benchmark/SiggraphPerformance/ring_spring_rest_length_one_timing.py#L56

[wsmoses]

I don't think I have access to that repo, @deomaiidae if you can add me

[martinjm97]

@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: https://github.com/EnzymeAD/Enzyme/pull/1889.

[wsmoses]

Yeah that post sparse todense is the one I remember working with you on during benchmarking, so that looks right.