Skip to content

tagging better #903

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

Merged
nicolasaunai merged 5 commits into from
Oct 18, 2024
Merged

tagging better #903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c994aa3
silent werror on ubuntu builds
nicolasaunai Oct 16, 2024
151e29f
debug03samrai
nicolasaunai Oct 16, 2024
3edcddc
Update cmake_ubuntu.yml
PhilipDeegan Oct 16, 2024
9495dc1
tagging better
nicolasaunai Oct 11, 2024
943c02f
cleaner
nicolasaunai Oct 16, 2024
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
4 changes: 2 additions & 2 deletions .github/workflows/cmake_ubuntu.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ jobs:
make -j2 && sudo make install && cd ../.. && rm -rf samrai
cd ${{runner.workspace}}/build && rm -rf *
cmake $GITHUB_WORKSPACE -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON --fresh \
-DCMAKE_BUILD_TYPE=RelWithDebInfo -Dasan=OFF \
-DCMAKE_BUILD_TYPE=Debug -Dasan=OFF \
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
-DlowResourceTests=ON -DdevMode=ON -Dbench=ON \
-DCMAKE_CXX_FLAGS="-DPHARE_DIAG_DOUBLES=1 " -Dphare_configurator=ON
-DCMAKE_CXX_FLAGS="-O3 -DPHARE_DIAG_DOUBLES=1 " -Dphare_configurator=ON
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Oct 16, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Reconsider the combination of Debug build type with -O3 optimization

The addition of the -O3 flag to CMAKE_CXX_FLAGS while using Debug build type is unusual and potentially problematic. This combination can lead to unexpected behavior as Debug builds typically include minimal optimizations to aid in debugging, while -O3 applies aggressive optimizations.

Consider one of the following alternatives:

  1. Remove the -O3 flag to maintain a true Debug build:
-              -DCMAKE_CXX_FLAGS="-O3 -DPHARE_DIAG_DOUBLES=1 " -Dphare_configurator=ON
+              -DCMAKE_CXX_FLAGS="-DPHARE_DIAG_DOUBLES=1 " -Dphare_configurator=ON
  1. Create a custom build type that enables both debugging symbols and optimizations:
-              -DCMAKE_BUILD_TYPE=RelWithDebInfo -Dasan=OFF \
+              -DCMAKE_BUILD_TYPE=CustomDebug -Dasan=OFF \
+              -DCMAKE_CXX_FLAGS_CUSTOMDEBUG="-g -O2 -DPHARE_DIAG_DOUBLES=1" \

This approach allows you to define a build type that suits your specific needs.

Committable suggestion was skipped due to low confidence.


- name: Build
working-directory: ${{runner.workspace}}/build
Expand Down
12 changes: 9 additions & 3 deletions src/amr/tagging/default_hybrid_tagger_strategy.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,15 @@ void DefaultHybridTaggerStrategy<HybridModel>::tag(HybridModel& model,
{
auto field_diff = [&](auto const& F) //
{
return std::make_tuple(
std::abs((F(ix + 2, iy) - F(ix, iy)) / (1 + F(ix + 1, iy) - F(ix, iy))),
std::abs((F(ix, iy + 2) - F(ix, iy)) / (F(ix, iy + 1) - F(ix, iy) + 1)));
auto const delta_2x = std::abs(F(ix + 2, iy) - F(ix, iy));
auto const delta_2y = std::abs(F(ix, iy + 2) - F(ix, iy));
auto const delta_x = std::abs(F(ix + 1, iy) - F(ix, iy));
auto const delta_y = std::abs(F(ix, iy + 1) - F(ix, iy));

auto const criter_x = delta_2x / (1 + delta_x);
auto const criter_y = delta_2y / (1 + delta_y);

return std::make_tuple(criter_x, criter_y);
};

auto const& [Bx_x, Bx_y] = field_diff(Bx);
Expand Down