Skip to content

Feature/215 remove the conditional benchmark code from the vts library - #216

Merged
lmalenfant merged 10 commits into
masterfrom
feature/215-remove-the-conditional-benchmark-code-from-the-vts-library
Jan 28, 2026
Merged

Feature/215 remove the conditional benchmark code from the vts library#216
lmalenfant merged 10 commits into
masterfrom
feature/215-remove-the-conditional-benchmark-code-from-the-vts-library

Conversation

@lmalenfant

Copy link
Copy Markdown
Member

Removed the conditional benchmark code from the production library and relocated it in the Benchmark application.

… created benchmark classes in the Benckmark project.
… accordingly. Added launch settings to allow passing the parallel argument.
@lmalenfant
lmalenfant requested a review from hayakawa16 January 23, 2026 21:00
@lmalenfant lmalenfant linked an issue Jan 23, 2026 that may be closed by this pull request
lmalenfant and others added 2 commits January 23, 2026 13:45
…o see if calculated mean is within 3-standard deviations of the prior mean (should happend 99.7% of the time), and modified header to indicate pull down now enables running Monte Carlo or parallel Monte Carlo in benchmark. A little code clean up.

@hayakawa16 hayakawa16 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I pulled latest updates, ran unit tests in Release build, ran the Vts.Benchmark and verified time of execution was within 3 standard deviation of prior run (this should occur 99.7% of the time), and ran the Vts.Benchmark parallel and verified time of execution was within 3 standard deviation of prior run.

I will re-review if @lmalenfant finds any updates that would be good to make after my last push.

…V file, for comparison with the new mean.

Created helper methods so they could be accessed multiple times and catch any errors if the CSV file is missing or open.
@lmalenfant
lmalenfant requested a review from hayakawa16 January 28, 2026 01:40
… to be the norm given it is 95.4%. Also added header Notes item to describe the use of prior run mean data as the validation mean if it exists, otherwise in prior mean defined in code is used.
@hayakawa16

Copy link
Copy Markdown
Member

Concerning the question of the mean for the parallel results with 4 CPUs being in units of ns and the mean for the non-parallel results being in units of ms, I ran both tests using N=10,000 instead of N=100, and now the mean for the parallel is 4.35ms and the non-parallel is 7.76ms. These numbers and corresponding units make more sense. So I think our using a small number of photons N=100 may cause the discrepancy in why the parallel is running in ns and the non-parallel is in ms. I am tempted to change the value of N in the code to 10,000. I would need to modify the default mean values and the tests run a little longer. Since I am the one running the tests I don't mind this.

…stimates. The units of both the mean values for the single MC (7.76ms) and parallel MC (4.35ms) are now in ms and make more sense. Curiously the single takes about 10 wallclock minutes to run and the parallel runs in under a minute. Not sure what overhead the benchmark software is doing to cause this difference. But since the estimated mean values are in comparable units, the results are more trusted.
@sonarqubecloud

Copy link
Copy Markdown

@hayakawa16

Copy link
Copy Markdown
Member

I'm putting my review here because regular option not apparent:
I modified to use 2 standard deviation checks since found that most other sites use this. I verified time of execution for vts. Benchmark was within 2 standard deviation of prior run (this should occur 95.4% of the time), and ran the Vts.Benchmark parallel and verified time of execution was within 2 standard deviation of prior run.

Everything looks good! I approve.

@lmalenfant
lmalenfant merged commit 508b5fe into master Jan 28, 2026
3 checks passed
@lmalenfant
lmalenfant deleted the feature/215-remove-the-conditional-benchmark-code-from-the-vts-library branch January 28, 2026 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove the conditional Benchmark code from the VTS library

2 participants