admb-project / admb

AD Model Builder
http://admb-project.org
Other
64 stars 19 forks source link

Performance issue in 13.0-beta #255

Closed arni-magnusson closed 2 years ago

arni-magnusson commented 2 years ago

Hi!

I've run some benchmarks that seem to indicate a performance issue in ADMB 13.0-beta:

https://github.com/admb-project/benchmarks

The results seem to indicate a general performance issue, not specific to hardware, OS, or models. Unless I'm doing something wrong, of course :)

arni-magnusson commented 2 years ago

A quick check with a Stock Synthesis model would be an important use case.

allanhicks commented 2 years ago

Hi Arni, Very interesting and important tests to do. I think that the SS team has run some benchmarks automatically using GitHub Actions, and saw encouraging results. You may want to view this interesting thread.

https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/257#issuecomment-1124950348

I haven’t run any tests, but Kathryn, Cole, Ian, Rick, Johnoel, and others would have more insight.

Good luck, Allan

On Sun, May 15, 2022 at 9:24 PM Arni Magnusson @.***> wrote:

A quick check with a Stock Synthesis model would be an important use case.

— Reply to this email directly, view it on GitHub https://github.com/admb-project/admb/issues/255#issuecomment-1127205040, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPA4ZJZZZZNUB2W3ZKQVZ3VKHEYVANCNFSM5WAAEXDA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Rick-Methot-NOAA commented 2 years ago

Support Allan's statement. Kathryn has run a suite of examples using SS3 that show ADMB 13.0 to be the same or slightly faster than 12.3. Rick

johnoel commented 2 years ago

Thanks Arni, for catching that. Please check again, it should be better.

johnoel commented 2 years ago

Used more low level c coding in #258.

arni-magnusson commented 2 years ago

Thanks for addressing the problem, Johnoel.

Unfortunately, the performance issue seems to be persist in Windows.

See old benchmark plot (May) and new benchmark plot (July).

Rick-Methot-NOAA commented 2 years ago

Arni, Your results are opposite of the performance Kathryn reported last week in which ADMB 13.0 was ~30% shorter runtimes than 12.3. Not sure if that was linux or windows.

k-doering-NOAA commented 2 years ago

The runs I did were on Linux, I haven't checked on Windows.

johnoel commented 2 years ago

Thanks Arni for rechecking, I have identified files to work on to improve the performance of those two examples. It is getting closer.

iantaylor-NOAA commented 2 years ago

One more thing to add on ADMB 13.0 performance. @k-doering-NOAA just did some tests with Stock Synthesis compiled with the fast option ("admb -f") and found that whereas with ADMB 12.3, the average model run with the fast executable took only 69% of the time as one compiled with the safe one, adding "-f" to when compiling in pre-release ADMB 13.0 slowed it down, with average run time at 133% of the safe version. More detail is at https://github.com/nmfs-stock-synthesis/stock-synthesis/issues/457 or from @k-doering-NOAA.

Some of this may be specific to Stock Synthesis, so it might be useful if @arni-magnusson can add a test of the fast option to his benchmarks.

Personally, I would much prefer performance improvements to the safe executable (as we've now seen on linux as discussed earlier in this issue). If the fast option isn't showing much benefit, I don't think it would be a big loss to leave it out of ADMB 13.0, but that's just me.

k-doering-NOAA commented 2 years ago

Thanks @iantaylor-NOAA , just adding for clarity that this was on linux.

johnoel commented 2 years ago

Hi @arni-magnusson, can you give it another check on your computers. I get pretty close results.