# Automated Performance Testing

**URL:** <https://openmc.discourse.group/t/automated-performance-testing/1178>\
**Category:** Development\
**Created:** [April 30, 2021, 3:50pm UTC](https://openmc.discourse.group/t/automated-performance-testing/1178 "2021-04-30T15:50:27Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![gridley](https://yyz2.discourse-cdn.com/free1/user_avatar/openmc.discourse.group/gridley/32/1064_2.png) [@gridley](https://openmc.discourse.group/u/gridley)\
**Post date:** [April 30, 2021, 3:50pm UTC](https://openmc.discourse.group/t/automated-performance-testing/1178/1 "2021-04-30T15:50:27Z")

</div>

> <https://github.com/openmc-dev/openmc/pull/1830#issuecomment-830163581>
>
> Fixes #1827 
> 
> So, I slipped up slightly when removing recursion. I had assumed… that if we try to iterate through neighbor lists, it will always touch the variable \`i\_cell\` in find\_cell\_inner. Well, that's actually not true, since it's possible that the neighbor lists are empty, which in that case, i\_cell will never be affected. As a result of this, we were never forming neighbor lists.
> 
> The fix to this is simple. If we're attempting a cell search based on neighbor lists, we should always return false if we failed to find the right cell in the neighbor list. This will cause an exhaustive search to happen, the result of which will be appended to the neighbor list.
> 
> Here is a simple 5x5 checkerboard input done directly using surfaces, so that the impact of neighbor lists is fairly high. On my computer, I saw the tracking rate go from around 190 kp/s to around 240 kp/s.
> 
> \`\`\`
> \<?xml version='1.0' encoding='utf-8'?\>
> \<geometry\>
> \<cell material="1" id="1" region="1 -5 3 -9" universe="0" /\>
> \<cell material="2" id="2" region="5 -6 3 -9" universe="0" /\>
> \<cell material="1" id="3" region="6 -7 3 -9" universe="0" /\>
> \<cell material="2" id="4" region="7 -8 3 -9" universe="0" /\>
> \<cell material="1" id="5" region="8 -2 3 -9" universe="0" /\>
> \<cell material="2" id="6" region="1 -5 9 -10" universe="0" /\>
> \<cell material="1" id="7" region="5 -6 9 -10" universe="0" /\>
> \<cell material="2" id="8" region="6 -7 9 -10" universe="0" /\>
> \<cell material="1" id="9" region="7 -8 9 -10" universe="0" /\>
> \<cell material="2" id="10" region="8 -2 9 -10" universe="0" /\>
> \<cell material="1" id="11" region="1 -5 10 -11" universe="0" /\>
> \<cell material="2" id="12" region="5 -6 10 -11" universe="0" /\>
> \<cell material="1" id="13" region="6 -7 10 -11" universe="0" /\>
> \<cell material="2" id="14" region="7 -8 10 -11" universe="0" /\>
> \<cell material="1" id="15" region="8 -2 10 -11" universe="0" /\>
> \<cell material="2" id="16" region="1 -5 11 -12" universe="0" /\>
> \<cell material="1" id="17" region="5 -6 11 -12" universe="0" /\>
> \<cell material="2" id="18" region="6 -7 11 -12" universe="0" /\>
> \<cell material="1" id="19" region="7 -8 11 -12" universe="0" /\>
> \<cell material="2" id="20" region="8 -2 11 -12" universe="0" /\>
> \<cell material="1" id="21" region="1 -5 12 -4" universe="0" /\>
> \<cell material="2" id="22" region="5 -6 12 -4" universe="0" /\>
> \<cell material="1" id="23" region="6 -7 12 -4" universe="0" /\>
> \<cell material="2" id="24" region="7 -8 12 -4" universe="0" /\>
> \<cell material="1" id="25" region="8 -2 12 -4" universe="0" /\>
> 
> \<surface boundary="reflective" coeffs="0" id="1" type="x-plane" /\>
> \<surface boundary="reflective" coeffs="10" id="2" type="x-plane" /\>
> \<surface boundary="reflective" coeffs="0" id="3" type="y-plane" /\>
> \<surface boundary="reflective" coeffs="10" id="4" type="y-plane" /\>
> 
> \<surface coeffs="2" id="5" type="x-plane" /\>
> \<surface coeffs="4" id="6" type="x-plane" /\>
> \<surface coeffs="6" id="7" type="x-plane" /\>
> \<surface coeffs="8" id="8" type="x-plane" /\>
> 
> \<surface coeffs="2" id="9" type="y-plane" /\>
> \<surface coeffs="4" id="10" type="y-plane" /\>
> \<surface coeffs="6" id="11" type="y-plane" /\>
> \<surface coeffs="8" id="12" type="y-plane" /\>
> \</geometry\>
> \<?xml version='1.0' encoding='utf-8'?\>
> \<materials\>
> \<material depletable="true" id="1" name="Fuel 3.1%" temperature="300"\>
> \<density units="g/cc" value="10.30166" /\>
> \<nuclide ao="1.9992419999999977" name="O16" /\>
> \<nuclide ao="0.0007579999999999991" name="O17" /\>
> \<nuclide ao="0.00028071399816198897" name="U234" /\>
> \<nuclide ao="0.03140630756586438" name="U235" /\>
> \<nuclide ao="0.9681691224623687" name="U238" /\>
> \<nuclide ao="0.0001438559736051798" name="U236" /\>
> \</material\>
> \<material id="2" name="Water" temperature="300"\>
> \<density units="g/cc" value="0.7405820675158279" /\>
> \<nuclide ao="0.0003217866044341838" name="B10" /\>
> \<nuclide ao="0.001301758322075321" name="B11" /\>
> \<nuclide ao="1.9964419358487546" name="H1" /\>
> \<nuclide ao="0.00031097429822629077" name="H2" /\>
> \<nuclide ao="0.9979980703970175" name="O16" /\>
> \<nuclide ao="0.00037838467647285283" name="O17" /\>
> \</material\>
> \</materials\>
> \<?xml version='1.0' encoding='utf-8'?\>
> \<plots\>
> \<plot basis="xy" color\_by="material" id="1" type="slice"\>
> \<origin\>5 5 0.5\</origin\>
> \<width\>10 10\</width\>
> \<pixels\>800 800\</pixels\>
> \</plot\>
> \</plots\>
> \<?xml version='1.0' encoding='utf-8'?\>
> \<settings\>
> \<run\_mode\>eigenvalue\</run\_mode\>
> \<particles\>100000\</particles\>
> \<batches\>5\</batches\>
> \<inactive\>3\</inactive\>
> \<source strength="1.0"\>
> \<space type="box"\>
> \<parameters\>0 10 0 10 1 1\</parameters\>
> \</space\>
> \</source\>
> \</settings\>
> 
> \`\`\`

As John pointed out here, we would have caught the regression I introduced that inhibits the formation of neighbor lists (now fixed) if we had automated performance testing. We can put ideas on implementing this and the details of it in this thread.

For instance… would we ever automatically fail a PR for a substantial performance degradation? I think it would make more sense to automatically create a report to show how the code before and after the merge runs on a few problems, preferably including a few things like fixed source mode, DagMC, WMP, etc.

It appears this is feasible in github actions:

[https://thomaspoignant.medium.com/ci-build-performance-testing-with-github-action-e6b227097c83](https://thomaspoignant.medium.com/ci-build-performance-testing-with-github-action-e6b227097c83)

---

<div class="post-metadata">

**Author:** ![paulromano](https://yyz2.discourse-cdn.com/free1/user_avatar/openmc.discourse.group/paulromano/32/486_2.png) [@paulromano](https://openmc.discourse.group/u/paulromano)\
**Post date:** [June 2, 2021, 4:02am UTC](https://openmc.discourse.group/t/automated-performance-testing/1178/2 "2021-06-02T04:02:16Z")

</div>

If we could capture something like that within github actions, it would be great, But it is a little constraining to run on the resources that are allotted (essentially 2 cores). My dream is to have a dedicated node with a fair number of cores so that we could tease out any potential performance issues with parallelism. That would likely require a more elaborate solution though. Running GH actions on a self-hosted runner for a public repo is [discouraged](https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#hardening-for-self-hosted-runners) due to the potential security issues. In lieu of that, we could have a manually triggered job that reports a status back to GitHub via its APIs.

---

<div class="post-metadata">

**Author:** ![gridley](https://yyz2.discourse-cdn.com/free1/user_avatar/openmc.discourse.group/gridley/32/1064_2.png) [@gridley](https://openmc.discourse.group/u/gridley)\
**Post date:** [June 4, 2021, 6:22pm UTC](https://openmc.discourse.group/t/automated-performance-testing/1178/3 "2021-06-04T18:22:26Z")

</div>

That’s definitely a solution. Another would be simply doing a CRON job on something like the NSEcluster; I’m thinking weekly. It could just log the commit it compiled on, and save performance results for a battery of tests. I think that would be pretty neat. This might be a reasonable approach, since minor performance degradations aren’t really a reason to turn away any PR. Rather, we just want to keep track of general trends over time.

Similarly, with your dedicated node approach, it could only do performance testing after a PR is accepted, not before. That should mitigate the security concern. The manual invocations could similarly be done if there’s a concern a given PR may affect performance.
