ACEsuit / ACE.jl

Parameterisation of Equivariant Properties of Particle Systems
66 stars 15 forks source link

Performance 2 - Linear Models #95

Closed andresrossb closed 2 years ago

andresrossb commented 2 years ago

benchmarking of linear models

cortner commented 2 years ago

done a bit of rewriting of rrule_evaluate. very promising. But still need to insert some final files before multiple properties works again as expected. Basically I want to take the opportunity of not only optimize the codes but also remove all the duplicate codes in the evaluator.

cortner commented 2 years ago

P.S.: there was another nasty performance bug in rrule_evaluate so we are definitely recovering signifant performance optimizations. Not quite the factor 3 of evaluate that I was hoping but definitely in the factor 4-5 range.

cortner commented 2 years ago

Andres - I had to change the test_multiprop.jl tests a bit. Would be worth it if you take a closer look there in case this propagates into your ACEflux. Basically, I now define contract(a::Number, b::AbstractVector) = a * b), which conflicts with how you've used it. But this is really important to allow a generic use-case of the adjoints...

cortner commented 2 years ago

@andresrossb should be ready for you to play :)

cortner commented 2 years ago

@andresrossb ... I'm trying to catch up again. What left to do here, just revisit the benchmark suite, and make sure it runs ok? At least on our AMD EPYC server?

cortner commented 2 years ago

I think

Pgroup["adjoint_EVAL_D1"] = BenchmarkGroup()

should be replaced and _D1 no longer exists, there should now be a single adjoint_EVAL_D.

cortner commented 2 years ago

Just asking the question for now, let's discuss: should the following be in ACE or elsewhere? E.g. ACEfit.jl?

#zygote calls
Pgroup["site_energy"] = BenchmarkGroup()
Pgroup["Zygote_grad_params"] = BenchmarkGroup()
Pgroup["Zygote_grad_config"] = BenchmarkGroup()
Pgroup["Zygote_grad_params_config"] = BenchmarkGroup()

#loss functions
Pgroup["eval_full_loss"] = BenchmarkGroup()
Pgroup["eval_energy_loss"] = BenchmarkGroup()
Pgroup["der_full_loss"] = BenchmarkGroup()
Pgroup["der_energy_loss"] = BenchmarkGroup()
cortner commented 2 years ago

maybe keep it to 1, 2, 4.

Pnumprops = [1, 2, 3, 4] 
cortner commented 2 years ago

I believe this is from my basis benchmarks?

# degrees = Dict(2 => [7, 12, 17],
#                3 => [7, 11, 15],
#                4 => [7, 10, 13], 
#                5 => [7, 9, 11] )

I'd like to reduce this a bit. How about

# degrees = Dict(2 => [10, 17],
#                3 => [9, 15],
#                5 => [7, 11] )

or even less than that .. otherwise the benchmark reports get too long.

andresrossb commented 2 years ago

Just asking the question for now, let's discuss: should the following be in ACE or elsewhere? E.g. ACEfit.jl?

#zygote calls
Pgroup["site_energy"] = BenchmarkGroup()
Pgroup["Zygote_grad_params"] = BenchmarkGroup()
Pgroup["Zygote_grad_config"] = BenchmarkGroup()
Pgroup["Zygote_grad_params_config"] = BenchmarkGroup()

#loss functions
Pgroup["eval_full_loss"] = BenchmarkGroup()
Pgroup["eval_energy_loss"] = BenchmarkGroup()
Pgroup["der_full_loss"] = BenchmarkGroup()
Pgroup["der_energy_loss"] = BenchmarkGroup()

gradient(m->sum(sum(gradient(x->site_energy(m,x), cfg)[1]).rr), LM) is not running on my end. There is a contract with a Chainrules.ZeroTangent() for some reason I don't yet understand. This is when running Pgroup["Zygote_grad_params_config"] = BenchmarkGroup()

andresrossb commented 2 years ago

Did bm_linear.jl run for you?

cortner commented 2 years ago

Didn’t try yet

cortner commented 2 years ago

I've made some changes to the benchmarks along the lines suggested above. The benchmarks now run and I think this represents a good first benchmark suite. If you are happy with those changes then we can merge. Happy to discuss though.

cortner commented 2 years ago

@andresrossb - when you have a moment can you let me know whether you are happy to merge this or wanted to do something else to finish this PR? Thank you.