95 implement embree runner #96

jmaack24 · 2026-01-13T18:07:34Z

Implementation of EmbreeRunner to utilize Intel's Embree ray tracing library (sold separately...).

…nner validation tests to include virtual stage

…nners

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 67 out of 68 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-15T21:05:57Z

coretrace/simulation_runner/embree_runner/embree_helper.cpp

+        // increment position by tiny amount to get off the element if 
+        // tracing to the same element


Fixed typo: 'increment' should start with capital 'I' to match comment style, and the comment should end with a period.

Suggested change

// increment position by tiny amount to get off the element if

// tracing to the same element

// Increment position by tiny amount to get off the element if

// tracing to the same element.

Copilot · 2026-01-15T21:05:57Z

coretrace/simulation_runner/embree_runner/bbox_calculator.hpp

+                    float (&min_coord_global)[3],
+                    float (&max_coord_global)[3]);
+
+} // namespace embree_helper


Namespace comment is incorrect. Should be '// namespace SolTrace::EmbreeRunner' to match the actual namespace declaration at line 8.

Suggested change

} // namespace embree_helper

} // namespace SolTrace::EmbreeRunner

Copilot · 2026-01-15T21:05:58Z

coretrace/simulation_runner/embree_runner/find_element_hit_embree.cpp

@@ -0,0 +1,90 @@
+#include "find_element_hit.hpp"


Incorrect include file name. Should be 'find_element_hit_embree.hpp' instead of 'find_element_hit.hpp'.

Suggested change

#include "find_element_hit.hpp"

#include "find_element_hit_embree.hpp"

Copilot · 2026-01-15T21:05:58Z

google-tests/unit-tests/simulation_data/aperture_test.cpp

+TEST(Aperture, BoundingBox)
+{
+}


Empty test case should either be implemented or removed. The test name suggests it should test aperture bounding box functionality but has no implementation.

Copilot · 2026-01-15T21:05:58Z

coretrace/simulation_data/surface.cpp

+        assert(is_approx(x_minmax[0], -r, 1e-6));
+        assert(is_approx(x_minmax[1], r, 1e-6));


Using assert for input validation in production code. Consider using a proper error handling mechanism or documenting that these are debug-only checks. Asserts are typically disabled in release builds.

Suggested change

assert(is_approx(x_minmax[0], -r, 1e-6));

assert(is_approx(x_minmax[1], r, 1e-6));

if (!is_approx(x_minmax[0], -r, 1e-6))

throw std::invalid_argument("Cylinder::bounding_box: x_minmax[0] must be approximately -radius");

if (!is_approx(x_minmax[1], r, 1e-6))

throw std::invalid_argument("Cylinder::bounding_box: x_minmax[1] must be approximately +radius");

Copilot · 2026-01-15T21:05:59Z

google-tests/regression-tests/native_runner_validation_test.cpp

                EXPECT_NE(rr->get_event(iidx), RayEvent::ABSORB);
                EXPECT_NE(rr->get_event(iidx), RayEvent::EXIT);
+
+                if(rr->get_event(iidx) == RayEvent::ABSORB)


Missing space after 'if' keyword. Should be 'if (rr->get_event(iidx) == RayEvent::ABSORB)'.

Copilot · 2026-01-15T21:05:59Z

coretrace/simulation_data/composite_element.cpp

            this->number_of_elements += el->get_number_of_elements();
            el->set_reference_element(this);
+            // Mark any added elements as virtual if needed
+            if(this->is_virtual())


Missing space after 'if' keyword. Should be 'if (this->is_virtual())'.

Suggested change

if(this->is_virtual())

if (this->is_virtual())

Copilot · 2026-01-15T21:05:59Z

coretrace/simulation_data/surface.cpp

+        // TODO: Fix ? This is really only the top half of the cylinder
+        //       Clyinder breaks the model since it is a mulit-valued fuction: each
+        //       x values produces two z values Returning only the positive root


Multiple spelling errors: 'Clyinder' should be 'Cylinder', 'mulit-valued fuction' should be 'multi-valued function', and missing period after 'root'.

Suggested change

// TODO: Fix ? This is really only the top half of the cylinder

// Clyinder breaks the model since it is a mulit-valued fuction: each

// x values produces two z values Returning only the positive root

// TODO: Fix ? This is really only the top half of the cylinder.

// Cylinder breaks the model since it is a multi-valued function: each

// x value produces two z values. Returning only the positive root.

Copilot

Pull request overview

Copilot reviewed 67 out of 68 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…windows CI

…cy code build by default

Copilot

Pull request overview

Copilot reviewed 70 out of 71 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-22T18:14:51Z

README.md


+## Steps for Building SolTrace (work in progress)
+
+SolTrace has been updated to use multiple ray tracing engines in addition to the prior implementation. Currently, there is no graphical user interface (it is underdevelopment).


The phrase "it is underdevelopment" should be "it is under development" (missing space).

Suggested change

SolTrace has been updated to use multiple ray tracing engines in addition to the prior implementation. Currently, there is no graphical user interface (it is underdevelopment).

SolTrace has been updated to use multiple ray tracing engines in addition to the prior implementation. Currently, there is no graphical user interface (it is under development).

Copilot · 2026-01-22T18:14:52Z

coretrace/simulation_runner/embree_runner/embree_helper.cpp

+        // Increment position by tiny amount to get off the element if 
+        // tracing to the same element.
+        PosRayElement[0] = PosRayElement[0] + 1.0e-4 * CosRayElement[0];
+        PosRayElement[1] = PosRayElement[1] + 1.0e-4 * CosRayElement[1];
+        PosRayElement[2] = PosRayElement[2] + 1.0e-4 * CosRayElement[2];


In the comment on line 126, "Increment position by tiny amount to get off the element" - this hardcoded offset of 1.0e-4 could cause issues with very small or very large geometries. Consider making this epsilon value scale-dependent or configurable based on the element's characteristic dimensions.

Copilot · 2026-01-22T18:14:52Z

coretrace/simulation_data/surface.cpp

+        // TODO: Fix ? This is really only the top half of the cylinder.
+        //       Cylinder breaks the model since it is a multi-valued function: each


The typo "Clyinder" should be "Cylinder" in the comment.

Copilot · 2026-01-22T18:14:52Z

.github/workflows/CI.yml

+    - name: Install embree (Windows)
+      if: matrix.os == 'windows-latest'
+      shell: pwsh
+      run: |
+        $embreeVersion = "4.4.0"
+        $embreeUrl = "https://github.com/embree/embree/releases/download/v$embreeVersion/embree-$embreeVersion.x64.windows.zip"
+        $embreeZip = "$env:TEMP\embree.zip"
+        $embreeDir = "C:\embree"
+        
+        Write-Host "Downloading Embree $embreeVersion..."
+        Invoke-WebRequest -Uri $embreeUrl -OutFile $embreeZip
+        
+        Write-Host "Extracting Embree..."
+        Expand-Archive -Path $embreeZip -DestinationPath $embreeDir -Force
+        
+        # $embreeRoot = Get-ChildItem -Path $embreeDir -Directory -Filter "embree*" | Select-Object -First 1
+        $embreeCMake = Join-Path $embreeDir "lib\cmake\embree-$embreeVersion"
+        $embreeLib = Join-Path $embreeDir "bin"
+        
+        # Write-Host "Embree root: $($embreeRoot.FullName)"
+        Write-Host "Embree CMake: $embreeCMake"
+        Write-Host "Embree Lib: $embreeLib"
+        
+        echo "embree_DIR=$embreeCMake" >> $env:GITHUB_ENV
+        echo "$embreeLib" >> $env:GITHUB_PATH


The Windows Embree installation script extracts to a directory path that includes the version number in the zip, but the code assumes a specific directory structure. The commented-out lines suggest there was an attempt to find the embree directory dynamically. This could break if Embree's directory structure changes between versions. Consider making the path detection more robust.

Copilot · 2026-01-22T18:14:53Z

coretrace/simulation_runner/embree_runner/bbox_calculator.cpp

+        // Expand bounding boxes slightly to account for float precision
+        const float expand = 1e-3f;
+        x_minmax[0] -= expand;
+        x_minmax[1] += expand;
+        y_minmax[0] -= expand;
+        y_minmax[1] += expand;
+        z_minmax[0] -= expand;
+        z_minmax[1] += expand;


The bounding box expansion with a hardcoded value of 1e-3f could be problematic for different scales. For very small geometries this could be too large, and for very large geometries it might not be sufficient. Consider making this relative to the bounding box dimensions or element scale.

coveralls · 2026-01-22T19:26:32Z

Pull Request Test Coverage Report for Build 21261701695

Details

1022 of 1268 (80.6%) changed or added relevant lines in 23 files are covered.
14 unchanged lines in 7 files lost coverage.
Overall coverage increased (+0.2%) to 88.052%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
coretrace/simulation_runner/native_runner/thread_manager.cpp	2	3	66.67%
coretrace/simulation_runner/native_runner/trace.cpp	8	9	88.89%
coretrace/simulation_data/surface.hpp	21	23	91.3%
coretrace/simulation_runner/native_runner/native_runner.cpp	16	18	88.89%
coretrace/simulation_runner/native_runner/sun_to_primary_stage.cpp	0	2	0.0%
coretrace/simulation_data/aperture.hpp	0	3	0.0%
coretrace/simulation_runner/embree_runner/embree_runner.cpp	36	40	90.0%
coretrace/simulation_data/utilities.hpp	7	12	58.33%
coretrace/simulation_data/surface.cpp	142	148	95.95%
coretrace/simulation_runner/native_runner/determine_interaction_type.cpp	17	26	65.38%

Files with Coverage Reduction	New Missed Lines	%
coretrace/simulation_data/aperture.hpp	1	74.19%
coretrace/simulation_runner/native_runner/native_runner.cpp	1	92.17%
coretrace/simulation_runner/native_runner/sphere_calculator.hpp	1	0.0%
coretrace/simulation_runner/native_runner/surface_intersection_calculator.hpp	1	50.0%
coretrace/simulation_runner/native_runner/trace.cpp	1	95.0%
coretrace/simulation_data/aperture.cpp	3	69.32%
coretrace/simulation_runner/native_runner/native_runner_types.cpp	6	93.84%

Totals
Change from base Build 21258149251:	0.2%
Covered Lines:	6721
Relevant Lines:	7633

💛 - Coveralls

jmaack24 added 5 commits January 8, 2026 16:12

Initial structure for EmbreeRunner

516450c

Bounding box calculations added to surface and aperture structures

6a4c716

Add tests for aperture bounding box

98dedec

Separate logging from thread manager

9d5e69c

Change embree runner to inherit from native runner

aa82d59

jmaack24 self-assigned this Jan 13, 2026

jmaack24 linked an issue Jan 13, 2026 that may be closed by this pull request

Implement Embree Runner #95

Open

jmaack24 added 9 commits January 13, 2026 11:16

Fix CI builds

944e179

Add tests for surface bounding box

7ea1104

Add logger to trace_embree function

6b47441

Add EmbreeRunner validation test and another short unit test

5ec6524

Set virtual flag for reading input files; update embree and native ru…

fc02507

…nner validation tests to include virtual stage

Add unit tests for thread manager

2d2b811

Move embree initialization to part of runner setup

e55fad9

Commonize interaction type determination between embree and native ru…

e4465f2

…nners

EmbreeRunner multi-threading implementation

9b52531

jmaack24 marked this pull request as ready for review January 15, 2026 19:40

jmaack24 requested a review from Copilot January 15, 2026 19:40

Copilot AI reviewed Jan 15, 2026

View reviewed changes

Copilot started reviewing on behalf of jmaack24 January 15, 2026 20:05 View session

jmaack24 requested a review from Copilot January 15, 2026 21:03

Copilot started reviewing on behalf of jmaack24 January 15, 2026 21:04 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

Address copilot comments

a5971e7

jmaack24 requested a review from Copilot January 15, 2026 21:18

Copilot started reviewing on behalf of jmaack24 January 15, 2026 21:19 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

jmaack24 requested review from qualand and taylorbrown75 January 15, 2026 21:46

Update CI and coverage work flows to build Embree

4a911fa

jmaack24 added 8 commits January 21, 2026 13:01

Fix work flow errors

878f6eb

Change windows build to use downloaded binary

6fe5cce

Attempt to fix windows build

73919c2

Fix windows CI hopefully. Maybe.

cf6f3c0

Pass along number of threads value to embree; another attempt to fix …

5050963

…windows CI

Fix linux compiler warning; attempt whatever to fix windows ci

c4ff4bf

Try again

079ff23

Updates to README for Embree install and SolTrace build; disable lega…

17afb27

…cy code build by default

jmaack24 requested a review from Copilot January 22, 2026 18:09

Copilot started reviewing on behalf of jmaack24 January 22, 2026 18:09 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

Merge branch 'develop' into 95-implement-embree-runner

8487343

		// increment position by tiny amount to get off the element if
		// tracing to the same element

	} // namespace embree_helper
	} // namespace SolTrace::EmbreeRunner

	#include "find_element_hit.hpp"
	#include "find_element_hit_embree.hpp"

		assert(is_approx(x_minmax[0], -r, 1e-6));
		assert(is_approx(x_minmax[1], r, 1e-6));

-        assert(is_approx(x_minmax[0], -r, 1e-6));
-        assert(is_approx(x_minmax[1], r, 1e-6));
+        if (!is_approx(x_minmax[0], -r, 1e-6))
+            throw std::invalid_argument("Cylinder::bounding_box: x_minmax[0] must be approximately -radius");
+        if (!is_approx(x_minmax[1], r, 1e-6))
+            throw std::invalid_argument("Cylinder::bounding_box: x_minmax[1] must be approximately +radius");


		## Steps for Building SolTrace (work in progress)

		SolTrace has been updated to use multiple ray tracing engines in addition to the prior implementation. Currently, there is no graphical user interface (it is underdevelopment).

		// TODO: Fix ? This is really only the top half of the cylinder.
		// Cylinder breaks the model since it is a multi-valued function: each

95 implement embree runner #96

Are you sure you want to change the base?

95 implement embree runner #96

Uh oh!

Conversation

jmaack24 commented Jan 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jan 22, 2026

Pull Request Test Coverage Report for Build 21261701695

Details

💛 - Coveralls

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants