Conversation
…ere the formatting is not applied
Add clang support and testing (add but don't run clang format)
This reverts commit 76f87d4.
Fix Include Dependencies and Standardize Formatting
| for (int i = 0; i < 3; ++i) | ||
| for (int j = 0; j < 3; ++j) EXPECT_NEAR(sum(i, j), eigSum(i, j), kThresh); |
There was a problem hiding this comment.
Use braces. A linter should also catch this.
| // Use a slightly looser tolerance for tests involving iterative methods (QR, Eigen) | ||
| // compared to the strict symmetry check tolerance. | ||
| #ifndef CONSTEIG_TEST_TOLERANCE | ||
| #define CONSTEIG_TEST_TOLERANCE 1e-9F |
There was a problem hiding this comment.
Is this actually looser?
| option(CONSTEIG_RAISE_COMPILER_LIMITS "Raise constexpr limits for all targets" OFF) | ||
| if(CONSTEIG_RAISE_COMPILER_LIMITS) | ||
| if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "GNU") | ||
| add_compile_options(-fconstexpr-ops-limit=1000000000) |
There was a problem hiding this comment.
This is here but it's also set for all the actual tests. Check if that's appropriate.
| if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") | ||
| add_compile_options(-fconstexpr-steps=1000000000) | ||
| endif() | ||
| add_compile_options(-fconstexpr-depth=1024) |
There was a problem hiding this comment.
Check this against the depth of the recursion limit in while loops in the constexpr functions.
| set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) | ||
| set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) |
There was a problem hiding this comment.
Do we even need these? We don't make libs.
|
|
||
| #ifndef CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE | ||
| #define CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE 1e-6 | ||
| #define CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE 1e-6 |
There was a problem hiding this comment.
This feels loosse? Check where it's used.
| std::complex<double> p1_c(poles_c(0, 0).real, poles_c(0, 0).imag); | ||
| std::complex<double> p2_c(poles_c(1, 0).real, poles_c(1, 0).imag); | ||
|
|
||
| std::complex<double> p1_d = std::exp(p1_c * T); |
There was a problem hiding this comment.
This should have a constexpr variant as well.
|
|
||
| // 1. Find Continuous-time Eigenvalues (Poles) using Consteig | ||
| // This calculates the poles directly from the system matrix. | ||
| consteig::Matrix<consteig::Complex<double>, 2, 1> poles_c = consteig::eigvals(A_c); |
There was a problem hiding this comment.
Shouldn't template type deduction figure out the sizing here without me needing to specify it?
There was a problem hiding this comment.
No but I think we can probably use auto with c++17 to figure it out.
1 participant
No description provided.