Skip to content

Transition to GLM#101

Merged
jmaack24 merged 25 commits intoNatLabRockies:developfrom
nicholasbl:dev_updates
Feb 16, 2026
Merged

Transition to GLM#101
jmaack24 merged 25 commits intoNatLabRockies:developfrom
nicholasbl:dev_updates

Conversation

@nicholasbl
Copy link
Contributor

@nicholasbl nicholasbl commented Jan 24, 2026

This hefty PR strips out the custom linear algebra library and replaces it with GLM, a fairly popular and high performance library. This is the first step toward using quaternions instead of a vector and rotation for user-facing simulation_data classes, as discussed with @qualand (Bill).

Aside from allowing some more traditional syntax, GLM improves the performance by around 10% on my machines, and opens the door to using AVX instructions explicitly for additional gains. However: debug builds perform slower, in some cases twice as slow due to the indirection GLM needs to provide platform specific optimizations.

This is a draft as the Optix test is crashing, but I'm not yet sure if it's my fault or not.

Changes:

  • Add in the cmake package manager CPM to make pulling in 3rd party libraries easier (wrapping FetchContent)
  • Move json library to CPM, and add in GLM
  • Replace all uses of Vector3d and Matrix3d with glm::dvec3 and glm::dmat3, and optimize usage

Notes:

  • Note that GLM vectors do have a [] operator, but it is more efficient to use .x, .y, .z accessors
  • GLM allows more idiomatic expressions
    • add_vector has been removed. These expressions can be made using normal syntax: variable = a * v + b * y
    • matrix * matrix and matrix * vector expressions can now be made using normal syntax
  • More optimizations can be made to make better use of SIMD
  • A small change was made to the Rectangle aperture to make is_in faster; this could be done for all apertures

@nicholasbl
Copy link
Contributor Author

nicholasbl commented Jan 24, 2026

@jmaack24 Bill suggested I ping you on this PR. This is a pretty large PR, so please let me know if I've messed something up.

@nicholasbl
Copy link
Contributor Author

A few tests failing here that didn't fail on my hardware. Looks like the debug ones are duration; that's expected due to the slow performance of GLM in debug mode. The release ones are failing from uninitialization, which I thought I had fixed, but I'll get that fixed up soon enough.

@coveralls
Copy link

coveralls commented Jan 24, 2026

Pull Request Test Coverage Report for Build 21916285546

Details

  • 913 of 970 (94.12%) changed or added relevant lines in 35 files are covered.
  • 71 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-1.2%) to 86.885%

Changes Missing Coverage Covered Lines Changed/Added Lines %
coretrace/simulation_data/sun.hpp 4 5 80.0%
coretrace/simulation_runner/embree_runner/trace_embree.cpp 29 30 96.67%
coretrace/simulation_data/json_helpers.hpp 2 4 50.0%
coretrace/simulation_data/aperture.cpp 23 27 85.19%
coretrace/simulation_data/vector_utility.hpp 0 4 0.0%
coretrace/simulation_runner/native_runner/generate_ray.cpp 13 17 76.47%
coretrace/simulation_data/element.hpp 6 11 54.55%
coretrace/simulation_data/matvec.cpp 36 42 85.71%
coretrace/simulation_runner/embree_runner/embree_helper.cpp 14 21 66.67%
coretrace/simulation_runner/native_runner/tracing_errors.cpp 41 52 78.85%
Files with Coverage Reduction New Missed Lines %
coretrace/simulation_data/surface.cpp 1 94.04%
coretrace/simulation_data/matvec.cpp 70 32.14%
Totals Coverage Status
Change from base Build 21458642404: -1.2%
Covered Lines: 6406
Relevant Lines: 7373

💛 - Coveralls

@nicholasbl
Copy link
Contributor Author

Struggling to get coverage to succeed. Getting weird negative counts.

@jmaack24
Copy link
Member

jmaack24 commented Feb 9, 2026

Struggling to get coverage to succeed. Getting weird negative counts.

Bill and I both ran into this. Something to do with the multi-threading (even though that's not supposed to be a problem). After trying to fix it, we eventually just turned off the multi-threaded tests in coverage.

Ignoring them is a good way to go for now. Maybe we can try to fix this after the beta release.

@nicholasbl nicholasbl marked this pull request as ready for review February 9, 2026 15:46
@nicholasbl
Copy link
Contributor Author

Looks like it worked this time. I think this is ready for review; I don't have permissions to add reviewers, however.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 106 out of 107 changed files in this pull request and generated 4 comments.


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

Copy link
Member

@jmaack24 jmaack24 left a comment

Choose a reason for hiding this comment

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

Looks good. No issues.

@nicholasbl
Copy link
Contributor Author

Excellent. Looks like we are all good to go.

@jmaack24 jmaack24 merged commit 0ab26d6 into NatLabRockies:develop Feb 16, 2026
11 checks passed
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.

3 participants

Comments