Skip to content

Optimize computation in GrowthCurveGaussian#3304

Merged
heplesser merged 5 commits intonest:masterfrom
heshpdx:master
Sep 5, 2024
Merged

Optimize computation in GrowthCurveGaussian#3304
heplesser merged 5 commits intonest:masterfrom
heshpdx:master

Conversation

@heshpdx
Copy link
Contributor

@heshpdx heshpdx commented Sep 3, 2024

Improves one of the hottest functions be reducing unnecessary computation. Removes one sqrt and three divs outside the loop, and two divs inside the loop. I measured about 10% speedup on my machine, depending on the workload.

  • Replace FP divides with FP multiplies. Most systems have divides slower than multiplies.
  • Pre-compute constant 1/(2*sqrt(ln(2))) instead of calculating it each time the function is called.

Can you please check my math? For some reason the spike recorder on structural_plasticity_benchmark.sli seems to be giving different output. My other workloads still produce the same results. Thanks!

Replace FP divides with FP multiplies. Pre-compute constants.
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heshpdx Thanks for spotting this. I added some comments pertaining to maths and style.

@heplesser heplesser added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Sep 3, 2024
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heshpdx Thanks for the update! It looks good now, I just have a tiny suggestion relating to naming, see below.

Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

heshpdx added a commit to heshpdx/nest-simulator that referenced this pull request Sep 3, 2024
Replace FP divides with FP multiplies and pre-compute loop invariants.
Similar to nest#3304
@heplesser heplesser merged commit a343246 into nest:master Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants

Comments