Skip to content

Fix distance mini-app BVH robustness #67

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.

Sign up for GitHub

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

Open
prudhomm wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix distance mini-app BVH robustness #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 62 additions & 39 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,37 @@ endif()
find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
find_package(pybind11 CONFIG REQUIRED)

find_package(Feel++ REQUIRED COMPONENTS Toolboxes PATHS ${FEELPP_DIR}/share/feelpp/feel/cmake/modules $ENV{FEELPP_DIR}/share/feelpp/feel/cmake/modules /usr/share/feelpp/feel/cmake/modules /usr/local/share/feelpp/feel/cmake/modules )
if(NOT FEELPP_FOUND)
message(FATAL_ERROR "Feel++ was not found on your system. Make sure to install it and specify the FEELPP_DIR to reference the installation directory.")
option(FEELPP_PROJECT_REQUIRE_TOOLBOXES "Require Feel++ Toolboxes to run cfg/json configurations" ON)

if ( NOT TARGET Feelpp::feelpp )
find_package(Feel++ CONFIG REQUIRED COMPONENTS Toolboxes

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Honor the opt-out before requiring Toolboxes

When configuring a core-only Feel++ install with -DFEELPP_PROJECT_REQUIRE_TOOLBOXES=OFF, this still asks find_package for REQUIRED COMPONENTS Toolboxes, so CMake fails before the later FEELPP_PROJECT_REQUIRE_TOOLBOXES check can allow the build. This makes the new option ineffective for standalone users who only want to build the non-toolbox apps; request the component conditionally or make the package lookup non-required for Toolboxes and let the explicit check decide.

Useful? React with 👍 / 👎.

PATHS
${FEELPP_DIR}/share/feelpp/feel/cmake/modules
$ENV{FEELPP_DIR}/share/feelpp/feel/cmake/modules
/usr/share/feelpp/feel/cmake/modules
/usr/local/share/feelpp/feel/cmake/modules
)
endif()

if ( NOT TARGET Feelpp::feelpp )
message(FATAL_ERROR "Feel++ target Feelpp::feelpp is not available.")
endif()
if ( NOT FEELPP_TOOLBOXES_FOUND )
message(WARNING "Feel++ Toolboxes SDK is not found on your system. The toolbox application is now disabled.")
if ( NOT COMMAND feelpp_add_application )
message(FATAL_ERROR "Feel++ CMake macro feelpp_add_application is not available.")
endif()

add_compile_options(-Wall -Werror -Wextra)
set(USE_FEELPP_TOOLBOXES OFF)
if ( FEELPP_HAS_TOOLBOXES OR FEELPP_TOOLBOXES_FOUND )
set(USE_FEELPP_TOOLBOXES ON)
endif()
if ( FEELPP_PROJECT_REQUIRE_TOOLBOXES AND NOT USE_FEELPP_TOOLBOXES )
message(FATAL_ERROR "Feel++ Toolboxes SDK is required to run the cfg/json configurations. Enable FEELPP_ENABLE_TOOLBOXES in-tree or install Feel++ with Toolboxes for standalone builds.")
endif()

option(FEELPP_PROJECT_ENABLE_WERROR "Treat warnings as errors in apps-feelpp" OFF)
if ( FEELPP_PROJECT_ENABLE_WERROR )
add_compile_options(-Werror)
endif()

enable_testing()

Expand All @@ -69,37 +91,38 @@ list(APPEND CPACK_SOURCE_IGNORE_FILES
.appveyor.yml
)

set(CPACK_PACKAGE_NAME "${PROJECT_NAME}-${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}${EXTRA_VERSION}")
set(CPACK_SOURCE_PACKAGE_FILE_NAME "${PROJECT_NAME}-${PROJECT_VERSION}${EXTRA_VERSION}")
SET(CPACK_PACKAGE_FILE_NAME "${PROJECT_NAME}-${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}${EXTRA_VERSION}-${CMAKE_SYSTEM_NAME}")
set(CPACK_PACKAGE_DIRECTORY "${PROJECT_BINARY_DIR}/assets")
set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Cemosis feelpp-app template application")
set(CPACK_PACKAGE_DESCRIPTION "Cemosis provides a Feel++ template application.")
set(CPACK_PACKAGE_VENDOR "Cemosis")
set(CPACK_PACKAGE_CONTACT "Christophe Prud'homme <christophe.prudhomme@cemosis.fr>")
set(CPACK_PACKAGE_VERSION_MAJOR "${PROJECT_VERSION_MAJOR}")
set(CPACK_PACKAGE_VERSION_MINOR "${PROJECT_VERSION_MINOR}")
set(CPACK_PACKAGE_VERSION_PATCH "${PROJECT_VERSION_PATCH}")


set(CPACK_GENERATOR "TGZ;DEB")
set(CPACK_SOURCE_GENERATOR "TGZ")


set(CPACK_DEBIAN_PACKAGE_NAME "${PROJECT_NAME}")
set(CPACK_DEBIAN_PACKAGE_MAINTAINER "${CPACK_PACKAGE_CONTACT}")
if ( USE_FEELPP_TOOLBOXES )
set(CPACK_DEBIAN_PACKAGE_DEPENDS "libfeelpp1 (>= 0.109)") # add depends with relevant toolbox lib
else()
set(CPACK_DEBIAN_PACKAGE_DEPENDS "libfeelpp1 (>= 0.109)")
if ( PROJECT_IS_TOP_LEVEL )
set(CPACK_PACKAGE_NAME "${PROJECT_NAME}-${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}${EXTRA_VERSION}")
set(CPACK_SOURCE_PACKAGE_FILE_NAME "${PROJECT_NAME}-${PROJECT_VERSION}${EXTRA_VERSION}")
SET(CPACK_PACKAGE_FILE_NAME "${PROJECT_NAME}-${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}${EXTRA_VERSION}-${CMAKE_SYSTEM_NAME}")
set(CPACK_PACKAGE_DIRECTORY "${PROJECT_BINARY_DIR}/assets")
set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Cemosis feelpp-app template application")
set(CPACK_PACKAGE_DESCRIPTION "Cemosis provides a Feel++ template application.")
set(CPACK_PACKAGE_VENDOR "Cemosis")
set(CPACK_PACKAGE_CONTACT "Christophe Prud'homme <christophe.prudhomme@cemosis.fr>")
set(CPACK_PACKAGE_VERSION_MAJOR "${PROJECT_VERSION_MAJOR}")
set(CPACK_PACKAGE_VERSION_MINOR "${PROJECT_VERSION_MINOR}")
set(CPACK_PACKAGE_VERSION_PATCH "${PROJECT_VERSION_PATCH}")


set(CPACK_GENERATOR "TGZ;DEB")
set(CPACK_SOURCE_GENERATOR "TGZ")


set(CPACK_DEBIAN_PACKAGE_NAME "${PROJECT_NAME}")
set(CPACK_DEBIAN_PACKAGE_MAINTAINER "${CPACK_PACKAGE_CONTACT}")
if ( USE_FEELPP_TOOLBOXES )
set(CPACK_DEBIAN_PACKAGE_DEPENDS "libfeelpp1 (>= 0.109)") # add depends with relevant toolbox lib
else()
set(CPACK_DEBIAN_PACKAGE_DEPENDS "libfeelpp1 (>= 0.109)")
endif()
set(CPACK_DEBIAN_PACKAGE_DESCRIPTION_SUMMARY "${CPACK_PACKAGE_DESCRIPTION_SUMMARY}")
set(CPACK_DEBIAN_PACKAGE_DESCRIPTION "${CPACK_PACKAGE_DESCRIPTION}")
set(CPACK_DEBIAN_PACKAGE_PRIORITY "optional")
set(CPACK_DEBIAN_PACKAGE_SECTION "science")
set(CPACK_DEBIAN_PACKAGE_VERSION "${CPACK_PACKAGE_VERSION_MAJOR}.${CPACK_PACKAGE_VERSION_MINOR}.${CPACK_PACKAGE_VERSION_PATCH}${EXTRA_VERSION}")
set(CPACK_DEBIAN_ARCHITECTURE "${CMAKE_SYSTEM_PROCESSOR}")
# set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_SOURCE_DIR}/cmake/debian/postinst")

include(CPack)
endif()
set(CPACK_DEBIAN_PACKAGE_DESCRIPTION_SUMMARY "${CPACK_PACKAGE_DESCRIPTION_SUMMARY}")
set(CPACK_DEBIAN_PACKAGE_DESCRIPTION "${CPACK_PACKAGE_DESCRIPTION}")
set(CPACK_DEBIAN_PACKAGE_PRIORITY "optional")
set(CPACK_DEBIAN_PACKAGE_SECTION "science")
set(CPACK_DEBIAN_PACKAGE_VERSION "${CPACK_PACKAGE_VERSION_MAJOR}.${CPACK_PACKAGE_VERSION_MINOR}.${CPACK_PACKAGE_VERSION_PATCH}${EXTRA_VERSION}")
set(CPACK_DEBIAN_ARCHITECTURE "${CMAKE_SYSTEM_PROCESSOR}")
# set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_SOURCE_DIR}/cmake/debian/postinst")

include(CPack)

9 changes: 8 additions & 1 deletion src/app-feelpp-distance/README.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ feelpp_app_distance [--h <h> --M <M> --gmsh.filename <path>]
- `M` is the number of ray casted for the BVH method
- `gmsh.filename` is the path to the geometry (or the mesh). This geometry should contain a physical marker named `boundary`.

Quick local smoke check with a coarse mesh and a small number of rays:

[source, bash]
----
feelpp_app_distance --gmsh.filename ./dist.geo --h 0.5 --M 8 --repository.append.np 0
----


[source, bash]
----
Expand All @@ -23,4 +30,4 @@ feelpp-benchmarking-exec --machine-config src/config/gaya/gaya.json --benchmark-
[source, bash]
----
feelpp-benchmarking-render --plot-configs src/app-feelpp-distance/plots.json --patch-reports gaya:all:all:all -sp
----
----
33 changes: 23 additions & 10 deletions src/app-feelpp-distance/distance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#include <iostream>

#include <algorithm>
#include <chrono>
#include <limits>
#include <stdexcept>
#include <fmt/chrono.h>
#include <feel/feelcore/environment.hpp>
#include <feel/feelcore/json.hpp>
Expand Down Expand Up @@ -84,6 +87,8 @@ int main(int argc, char**argv )
double h_ = doption(_name="h");
int M = ioption(_name="M");
std::string filename = soption(_name="gmsh.filename");
if ( M <= 0 )
throw std::invalid_argument( "M must be a positive number of rays" );

// Load the mesh
tic();
Expand Down Expand Up @@ -127,8 +132,8 @@ int main(int argc, char**argv )
Eigen::Vector3d origin;
std::unordered_set<size_type> pointIDs;

std::vector<Points_list> origins(Xh_->nLocalDof());
size_type next_origin = 0;
std::vector<Points_list> origins;
origins.reserve(Xh_->nLocalDof());

for ( auto const& eltWrap : elements(mesh_) ) // on parcourt tous les éléments du maillage
{
Expand All @@ -138,17 +143,17 @@ int main(int argc, char**argv )
{
auto const & point = elt.point(p);

// pas sure si isGhostCell est la bonne fonction
if (!point.isOnBoundary() && !point.isGhostCell()) // on ne considère pas les points qui se trouvent au bord ni les dof ghosts
// Keep shared/ghost dofs here: local element evaluations need those
// values even when the dof owner is another rank.
if (!point.isOnBoundary())
{
size_type id_p = Xh_->dof()->localToGlobal( elt.id(), p ).index();
auto [it,inserted] = pointIDs.insert(id_p);

if (inserted)
{
origin << point.node()[0], point.node()[1], point.node()[2];
origins[next_origin] = Points_list{id_p, origin};
next_origin++;
origins.push_back(Points_list{id_p, origin});
}
}
}
Expand Down Expand Up @@ -180,15 +185,23 @@ int main(int argc, char**argv )
//toc("intersection");
//tic();
// Get distance
std::vector<double> dist(M, 0.0);
std::vector<double> dist(M, std::numeric_limits<double>::max());

for (auto const& [fid,rirs] : enumerate(multiRayIntersectionResult))
{
int rayIndex = fid % M;
if ( rayIndex == 0 )
std::fill(dist.begin(), dist.end(), std::numeric_limits<double>::max());

if (!rirs.empty()) // on check l'intersection
dist[fid % M] = rirs.front().distance();
dist[rayIndex] = rirs.front().distance();

if (fid % M == M - 1)
d_BVH[origins[static_cast<int>(fid / M)].id] = *(std::min_element(dist.begin(), dist.end()));
if ( rayIndex == M - 1 )
{
auto minDistance = *(std::min_element(dist.begin(), dist.end()));
if ( minDistance < std::numeric_limits<double>::max() )
d_BVH[origins[static_cast<int>(fid / M)].id] = minDistance;
}
}
double time_BVH = toc("bvh");
e_->add( "d_BVH", d_BVH );
Expand Down
Loading