-
Notifications
You must be signed in to change notification settings - Fork 54
Issue 525 FE Basis refactor implementation #561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
zasexton
wants to merge
40
commits into
SimVascular:main
Choose a base branch
from
zasexton:issue-525
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
4e47613
Merge pull request #3 from SimVascular/main
zasexton 3bdfb91
Merge branch 'SimVascular:main' into main
zasexton 3fb5364
Merge branch 'SimVascular:main' into main
zasexton 4749c70
Merge branch 'SimVascular:main' into main
zasexton 47a7985
Merge branch 'SimVascular:main' into main
zasexton 2d4f8ed
Merge pull request #5 from SimVascular/main
zasexton 1154fe8
Merge branch 'SimVascular:main' into main
zasexton 690a92c
Merge branch 'SimVascular:main' into main
zasexton e6dd7e9
Merge branch 'SimVascular:main' into main
zasexton b831ea1
Merge branch 'SimVascular:main' into main
zasexton 1e117c2
Merge branch 'SimVascular:main' into main
zasexton 35cc8fe
Merge branch 'SimVascular:main' into main
zasexton 42b23df
Merge branch 'SimVascular:main' into main
zasexton f412876
Merge branch 'SimVascular:main' into main
zasexton 3a6af11
Merge branch 'SimVascular:main' into main
zasexton 0ff0d30
#525 adding LagrangeBasis/Serendipity function support and unit tests…
zasexton dfdeead
Update FSI HEX8 FE Basis reference results
zasexton 8b47802
fixing temporary A + B expression in matrix and vector objects
zasexton 4d6baaa
fixing fetch content for google tests
zasexton 81cad54
adding fetch content to include for enabled unit tests
zasexton 004e678
removing basis optimizations, caching, pyramid support, manual/static…
zasexton 3876ee1
removing prewarmed evaluations and switch to std library constants. r…
zasexton 2a97fa0
consolidating math support for integer functions and expression opera…
zasexton 7f2e020
removing the previous basis functions so that we are not maintaining …
zasexton 6e31724
Merge branch 'main' into issue-525
zasexton 36046f8
fixing the licensing and copyright comments
zasexton ab0ebcc
Merge branch 'main' into issue-525
zasexton 3691503
including doxygen documentation for Basis and Math submodules
zasexton c53e0e0
updating serendipity basis to be concrete terminal classes with `final`
zasexton 1289c08
adding switch cases for converting consts element types to fe element…
zasexton 82a1158
adding doxygen to Common submodule
zasexton 917c638
aligning exception throws and raises with the function-template calls…
zasexton 4819f59
fixing doxygen layout to allow for visible topic sections since modul…
zasexton dfd5aff
added topology evaluation helpers and cleaned up static cast helpers
zasexton ddb509a
aligning throw and raise to use function-template helpers for svmp
zasexton 9d6266b
improving doxygen documentation for the basis topic
zasexton 3e03138
Merge branch 'main' into issue-525
zasexton bd7c2ad
removing chrono guard from Eigen
zasexton 2826269
reverting chrono replacement code changes
zasexton f734094
swapping out raw pointers for span support in the non-owning buffer a…
zasexton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,15 +23,18 @@ | |
| # LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING | ||
| # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS | ||
|
|
||
| set(CMAKE_CXX_STANDARD 17) | ||
| set(CMAKE_CXX_STANDARD 20) | ||
| set(CMAKE_CXX_STANDARD_REQUIRED True) | ||
|
|
||
| include_directories(${SV_SOURCE_DIR}/ThirdParty/eigen/include) | ||
| include_directories(${SV_SOURCE_DIR}/ThirdParty/eigen/include/eigen3) | ||
| include_directories(${SV_SOURCE_DIR}/ThirdParty/parmetis_internal/simvascular_parmetis_internal/ParMETISLib) | ||
| include_directories(${SV_SOURCE_DIR}/ThirdParty/tetgen/simvascular_tetgen) | ||
| include_directories(${SV_SOURCE_DIR}/ThirdParty/tinyxml/simvascular_tinyxml) | ||
| include_directories(${MPI_C_INCLUDE_PATH}) | ||
| include_directories(${CMAKE_CURRENT_SOURCE_DIR}) | ||
| include_directories(${CMAKE_CURRENT_SOURCE_DIR}/Core) | ||
| include_directories(${CMAKE_CURRENT_SOURCE_DIR}/FE) | ||
| include_directories(${CMAKE_CURRENT_SOURCE_DIR}/FE/Common) | ||
|
|
||
| # Find Trilinos package if requested | ||
|
|
@@ -86,7 +89,7 @@ endif() | |
| # add trilinos flags and defines | ||
| if(USE_TRILINOS) | ||
| ADD_DEFINITIONS(-DWITH_TRILINOS) | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20") | ||
|
Comment on lines
-89
to
+92
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the above, but also: would it make sense to take this version number from the CMake variable |
||
| endif() | ||
|
|
||
| # Build with the PETSc linear algebra package. | ||
|
|
@@ -245,9 +248,21 @@ file(GLOB SOLVER_FE_COMMON_SRCS CONFIGURE_DEPENDS | |
| FE/Common/*.h | ||
| ) | ||
|
|
||
| file(GLOB SOLVER_FE_BASIS_SRCS CONFIGURE_DEPENDS | ||
| FE/Basis/*.cpp | ||
| FE/Basis/*.h | ||
| ) | ||
|
|
||
| file(GLOB SOLVER_FE_MATH_SRCS CONFIGURE_DEPENDS | ||
| FE/Math/*.cpp | ||
| FE/Math/*.h | ||
| ) | ||
|
|
||
| list(APPEND CSRCS | ||
| ${SOLVER_CORE_SRCS} | ||
| ${SOLVER_FE_COMMON_SRCS} | ||
| ${SOLVER_FE_BASIS_SRCS} | ||
| ${SOLVER_FE_MATH_SRCS} | ||
| ) | ||
|
|
||
| # Set PETSc interace code. | ||
|
|
@@ -324,16 +339,24 @@ if(ENABLE_UNIT_TEST) | |
|
|
||
| # link pthread on ubuntu20 | ||
| find_package(Threads REQUIRED) | ||
|
|
||
| include(FetchContent) | ||
| # install Google Test | ||
| #if(NOT TARGET gtest_main AND NOT TARGET gtest) | ||
| include(FetchContent) | ||
| FetchContent_Declare( | ||
| googletest | ||
| URL https://github.com/google/googletest/archive/refs/heads/main.zip | ||
| DOWNLOAD_EXTRACT_TIMESTAMP TRUE | ||
| googletest | ||
| GIT_REPOSITORY https://github.com/google/googletest.git | ||
| GIT_TAG v1.17.0 | ||
| DOWNLOAD_EXTRACT_TIMESTAMP TRUE | ||
| ) | ||
| FetchContent_MakeAvailable(googletest) | ||
|
|
||
| if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_STANDARD GREATER_EQUAL 20) | ||
| foreach(GTEST_TARGET gtest gtest_main gmock gmock_main) | ||
| if(TARGET ${GTEST_TARGET}) | ||
| target_compile_options(${GTEST_TARGET} PRIVATE -std=gnu++17) | ||
| endif() | ||
| endforeach() | ||
| endif() | ||
| #endif() | ||
|
|
||
| enable_testing() | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) Stanford University, The Regents of the University of California, and others. | ||
| // SPDX-License-Identifier: BSD-3-Clause | ||
|
|
||
| #ifndef SVMP_FE_BASIS_BASISEXCEPTIONS_H | ||
| #define SVMP_FE_BASIS_BASISEXCEPTIONS_H | ||
|
|
||
| #include "FEException.h" | ||
|
|
||
| namespace svmp { | ||
| namespace FE { | ||
| namespace basis { | ||
|
|
||
| /** | ||
| * @brief Base exception type for errors originating in the Basis module | ||
| */ | ||
| class BasisException : public FEException { | ||
| public: | ||
| BasisException(const std::string& message, | ||
| const char* file = "", | ||
| int line = 0, | ||
| const char* function = "", | ||
| StatusCode status = StatusCode::Unknown) | ||
| : FEException(message, status, file, line, function) {} | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Invalid Basis request or configuration | ||
| */ | ||
| class BasisConfigurationException : public BasisException { | ||
| public: | ||
| BasisConfigurationException(const std::string& message, | ||
| const char* file = "", | ||
| int line = 0, | ||
| const char* function = "") | ||
| : BasisException(message, file, line, function, StatusCode::InvalidArgument) {} | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Requested element topology is incompatible with the basis family | ||
| */ | ||
| class BasisElementCompatibilityException : public BasisException { | ||
| public: | ||
| BasisElementCompatibilityException(const std::string& message, | ||
| const char* file = "", | ||
| int line = 0, | ||
| const char* function = "") | ||
| : BasisException(message, file, line, function, StatusCode::InvalidArgument) {} | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Basis evaluation request cannot be satisfied | ||
| */ | ||
| class BasisEvaluationException : public BasisException { | ||
| public: | ||
| BasisEvaluationException(const std::string& message, | ||
| const char* file = "", | ||
| int line = 0, | ||
| const char* function = "") | ||
| : BasisException(message, file, line, function, StatusCode::InvalidArgument) {} | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Public-to-canonical node ordering or coordinate lookup failure | ||
| */ | ||
| class BasisNodeOrderingException : public BasisException { | ||
| public: | ||
| BasisNodeOrderingException(const std::string& message, | ||
| const char* file = "", | ||
| int line = 0, | ||
| const char* function = "") | ||
| : BasisException(message, file, line, function, StatusCode::InvalidArgument) {} | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Internal basis construction or transform setup failure | ||
| */ | ||
| class BasisConstructionException : public BasisException { | ||
| public: | ||
| BasisConstructionException(const std::string& message, | ||
| const char* file = "", | ||
| int line = 0, | ||
| const char* function = "") | ||
| : BasisException(message, file, line, function, StatusCode::InternalError) {} | ||
| }; | ||
|
|
||
| } // namespace basis | ||
| } // namespace FE | ||
| } // namespace svmp | ||
|
|
||
| #endif // SVMP_FE_BASIS_BASISEXCEPTIONS_H |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) Stanford University, The Regents of the University of California, and others. | ||
| // SPDX-License-Identifier: BSD-3-Clause | ||
|
|
||
| #include "BasisFactory.h" | ||
|
|
||
| #include "BasisTraits.h" | ||
| #include "LagrangeBasis.h" | ||
| #include "SerendipityBasis.h" | ||
|
|
||
| namespace svmp { | ||
| namespace FE { | ||
| namespace basis { | ||
|
|
||
| namespace { | ||
|
|
||
| int require_basis_order(const BasisRequest& req, | ||
| const char* missing_message, | ||
| const char* negative_message) { | ||
| FE::throw_if<BasisConfigurationException>(!req.order.has_value(), SVMP_HERE, | ||
| missing_message); | ||
| FE::throw_if<BasisConfigurationException>(*req.order < 0, SVMP_HERE, | ||
| negative_message); | ||
| return *req.order; | ||
| } | ||
|
|
||
| void require_scalar_c0_request(const BasisRequest& req) { | ||
| FE::throw_if<BasisConfigurationException>( | ||
| req.field_type != FieldType::Scalar, SVMP_HERE, | ||
| "BasisFactory: Lagrange/Serendipity bases support scalar fields only"); | ||
| FE::throw_if<BasisConfigurationException>( | ||
| req.continuity != Continuity::C0, SVMP_HERE, | ||
| "BasisFactory: Lagrange/Serendipity bases support C0 continuity only"); | ||
| } | ||
|
|
||
| std::shared_ptr<BasisFunction> create_lagrange(const BasisRequest& req) { | ||
| require_scalar_c0_request(req); | ||
| const int order = require_basis_order( | ||
| req, | ||
| "BasisFactory: Lagrange creation requires an explicit order", | ||
| "BasisFactory: Lagrange requires non-negative order"); | ||
| return std::make_shared<LagrangeBasis>(req.element_type, order); | ||
| } | ||
|
|
||
| std::shared_ptr<BasisFunction> create_serendipity(const BasisRequest& req) { | ||
| require_scalar_c0_request(req); | ||
| const int order = require_basis_order( | ||
| req, | ||
| "BasisFactory: Serendipity creation requires an explicit order", | ||
| "BasisFactory: Serendipity requires non-negative order"); | ||
| return std::make_shared<SerendipityBasis>(req.element_type, order); | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| namespace basis_factory { | ||
|
|
||
| std::shared_ptr<BasisFunction> create(const BasisRequest& req) { | ||
| switch (req.basis_type) { | ||
| case BasisType::Lagrange: | ||
| return create_lagrange(req); | ||
| case BasisType::Serendipity: | ||
| return create_serendipity(req); | ||
| default: | ||
| FE::raise<BasisConfigurationException>(SVMP_HERE, | ||
| "BasisFactory: requested basis family is outside the scalar Lagrange/Serendipity scope"); | ||
| } | ||
| } | ||
|
|
||
| BasisRequest default_basis_request(ElementType element_type) { | ||
| switch (element_type) { | ||
| // Reduced serendipity node layouts have no complete Lagrange basis at | ||
| // their node count; they always use the quadratic serendipity space. | ||
| case ElementType::Quad8: | ||
| case ElementType::Hex20: | ||
| case ElementType::Wedge15: | ||
| return BasisRequest{element_type, BasisType::Serendipity, 2}; | ||
| case ElementType::Point1: | ||
| return BasisRequest{element_type, BasisType::Lagrange, 0}; | ||
| default: { | ||
| const int order = complete_lagrange_alias_order(element_type); | ||
| if (order >= 0) { | ||
| return BasisRequest{element_type, BasisType::Lagrange, order}; | ||
| } | ||
| FE::raise<BasisElementCompatibilityException>(SVMP_HERE, | ||
| "BasisFactory: no default basis is defined for the requested element type"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| std::shared_ptr<BasisFunction> create_default_for(ElementType element_type) { | ||
| return create(default_basis_request(element_type)); | ||
| } | ||
|
|
||
| } // namespace basis_factory | ||
|
|
||
| } // namespace basis | ||
| } // namespace FE | ||
| } // namespace svmp |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) Stanford University, The Regents of the University of California, and others. | ||
| // SPDX-License-Identifier: BSD-3-Clause | ||
|
|
||
| #ifndef SVMP_FE_BASIS_BASISFACTORY_H | ||
| #define SVMP_FE_BASIS_BASISFACTORY_H | ||
|
|
||
| /** | ||
| * @file BasisFactory.h | ||
| * @brief Runtime creation of basis families | ||
| */ | ||
|
|
||
| #include "BasisFunction.h" | ||
| #include <memory> | ||
| #include <optional> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| namespace svmp { | ||
| namespace FE { | ||
| namespace basis { | ||
|
|
||
| struct BasisRequest { | ||
| ElementType element_type; | ||
| BasisType basis_type; | ||
| std::optional<int> order{}; | ||
| Continuity continuity{Continuity::C0}; | ||
| FieldType field_type{FieldType::Scalar}; | ||
| std::vector<Real> knot_vector{}; | ||
| std::vector<Real> weights{}; | ||
| std::vector<int> axis_orders{}; | ||
| std::vector<std::vector<Real>> axis_knot_vectors{}; | ||
| std::vector<std::vector<Real>> axis_weights{}; | ||
| std::vector<int> tensor_extents{}; | ||
| std::string custom_id{}; | ||
| }; | ||
|
|
||
| namespace basis_factory { | ||
|
|
||
| [[nodiscard]] std::shared_ptr<BasisFunction> create(const BasisRequest& req); | ||
|
|
||
| /// \brief Return the default basis request (family and order) for an element type. | ||
| /// | ||
| /// \details This is the single source of truth for which basis family and | ||
| /// polynomial order a given element type uses by default: serendipity node | ||
| /// layouts (Quad8, Hex20, Wedge15) select the quadratic serendipity family, | ||
| /// and every complete Lagrange element selects the Lagrange family at the | ||
| /// order given by its node layout. Solver-facing adapters should translate | ||
| /// their element names to ElementType and delegate the basis choice here | ||
| /// rather than tabulating family/order themselves. | ||
| /// | ||
| /// \param element_type Element type to select a default basis for. | ||
| /// \return Basis request suitable for create(). | ||
| /// \throws BasisElementCompatibilityException If no default basis is defined | ||
| /// for the element type. | ||
| [[nodiscard]] BasisRequest default_basis_request(ElementType element_type); | ||
|
|
||
| /// \brief Create the default basis for an element type. | ||
| /// | ||
| /// \details Equivalent to create(default_basis_request(element_type)). | ||
| /// | ||
| /// \param element_type Element type to create a default basis for. | ||
| /// \return Shared basis instance. | ||
| [[nodiscard]] std::shared_ptr<BasisFunction> create_default_for(ElementType element_type); | ||
|
|
||
| } // namespace basis_factory | ||
|
|
||
| } // namespace basis | ||
| } // namespace FE | ||
| } // namespace svmp | ||
|
|
||
| #endif // SVMP_FE_BASIS_BASISFACTORY_H |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What features from C++ 20 are required now?
I don't know about recent updates, but until last year or so compiler support for C++ 20 was still a bit sketchy, and requiring developers to update to the latest compiler versions might be a big ask, especially in HPC environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm if this is the case then I would probably be adding more raw pointer handling back into some of this code. I've been using
std::spanfor some of these to offer a safer non-owning view over a contiguous sequence of objects.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this note, I could make a helper
spanlike work around to place in the FE/Common folder.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zasexton I see that C++ 20 has wide support; now the default for the latest GCC compilers, and the Intel compiler uses Clang/LLVM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then if this is okay with you @ktbolt I will continue to use
std::spanand clean up some of the cmake plumbing as @michelebucelli points out. I believe that it would be great to move to c++20 support if compiler architectures now have wide support.