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.

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

95 implement embree runner #96

95 implement embree runner #96

Uh oh!

jmaack24 commented Jan 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI Jan 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// 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");

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants