[SPH][GSPH][Common] Make the rendering module of SPH common to SPH and GSPH#1717
Draft
y-lapeyre wants to merge 3 commits intoShamrock-code:mainfrom
Draft
[SPH][GSPH][Common] Make the rendering module of SPH common to SPH and GSPH#1717y-lapeyre wants to merge 3 commits intoShamrock-code:mainfrom
y-lapeyre wants to merge 3 commits intoShamrock-code:mainfrom
Conversation
Contributor
|
Thanks @y-lapeyre for opening this PR! You can do multiple things directly here: Once the workflow completes a message will appear displaying informations related to the run. Also the PR gets automatically reviewed by gemini, you can: |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring effort to establish a common rendering module for the SPH and GSPH solvers. By relocating rendering-specific code to a shared Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the rendering module to make it common for both SPH and GSPH, which is a great step towards reducing code duplication. The main changes involve moving rendering files to a common directory, introducing a RenderConfig struct, and templating the renderer on the SolverStorage type.
My review highlights a few critical issues that would prevent compilation, such as a missing header include and an unqualified function call. I've also commented on the circular dependency issue you mentioned in the description, suggesting a standard approach to resolve it by moving the explicit template instantiations. Additionally, I've included some minor cleanup suggestions for code style and comments to improve maintainability.
|
|
||
| #include "shammodels/sph/modules/render/RenderFieldGetter.hpp" | ||
| #include "shammodels/sph/math/density.hpp" | ||
| #include "shammodels/common/modules/render/RenderFieldGetter.hpp" |
Contributor
There was a problem hiding this comment.
This file uses the rho_h function, but it doesn't include the necessary header shammodels/common/density.hpp where this function is declared (in the shamrock::sph namespace). This will likely cause a compilation error.
Suggested change
| #include "shammodels/common/modules/render/RenderFieldGetter.hpp" | |
| #include "shammodels/common/modules/render/RenderFieldGetter.hpp" | |
| #include "shammodels/common/density.hpp" |
| u32 gid = (u32) item.get_id(); | ||
| using namespace shamrock::sph; | ||
|
|
||
| Tscal rho_ha = rho_h(part_mass, acc_h[gid], Kernel::hfactd); |
Contributor
There was a problem hiding this comment.
The call to rho_h is unqualified. Since the using namespace shamrock::sph; directive was removed from this block, this will fail to compile. Please qualify the call with shamrock::sph::rho_h.
Suggested change
| Tscal rho_ha = rho_h(part_mass, acc_h[gid], Kernel::hfactd); | |
| Tscal rho_ha = shamrock::sph::rho_h(part_mass, acc_h[gid], Kernel::hfactd); |
Comment on lines
+622
to
+653
| template class shammodels::common::modules::CartesianRender<f64_3, f64, M4, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, M6, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, M8, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
|
|
||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, C2, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, C4, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, C6, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
|
|
||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M4, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M6, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M8, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
|
|
||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C2, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C4, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C6, shammodels::sph::SolverStorage<f64_3, u32>>; | ||
|
|
||
| //gsph | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, M4, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, M6, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, M8, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
|
|
||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, C2, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, C4, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64, C6, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
|
|
||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M4, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M6, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M8, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
|
|
||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C2, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C4, shammodels::gsph::SolverStorage<f64_3, u32>>; | ||
| template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C6, shammodels::gsph::SolverStorage<f64_3, u32>>; No newline at end of file |
Contributor
There was a problem hiding this comment.
As you noted in the PR description, these explicit template instantiations create a circular dependency. The shammodels_common library now depends on shammodels_sph and shammodels_gsph, while those libraries will depend on common. To resolve this, you should move these instantiations out of the common module. The sph instantiations should go into a new .cpp file within the sph module, and the gsph instantiations into a file within the gsph module. This will break the circular dependency between the libraries.
Comment on lines
+23
to
+32
| template<class Tscal> | ||
| struct RenderConfig; | ||
|
|
||
| template<class Tscal> | ||
| struct RenderConfig { | ||
| //Tscal hfact; | ||
| Tscal gpart_mass; | ||
| unsigned int tree_reduction_level; | ||
|
|
||
| }; |
Contributor
There was a problem hiding this comment.
The forward declaration template<class Tscal> struct RenderConfig; on line 24 is redundant since the struct is defined immediately after. Also, the struct definition has inconsistent indentation. I'd also recommend adding a newline at the end of the file for consistency with other files.
template<class Tscal>
struct RenderConfig {
//Tscal hfact;
Tscal gpart_mass;
unsigned int tree_reduction_level;
};| sycl::range<1>{pdat.get_obj_cnt()}, [=](sycl::item<1> item) { | ||
| u32 gid = (u32) item.get_id(); | ||
| using namespace shamrock::sph; | ||
| using namespace sph; |
Contributor
There was a problem hiding this comment.
using namespace sph; seems to be a typo for using namespace shamrock::sph;. However, it doesn't seem to be used in this block, so it can be removed to improve clarity.
| @@ -619,18 +619,35 @@ namespace shammodels::sph::modules { | |||
| } // namespace shammodels::sph::modules | |||
Contributor
There was a problem hiding this comment.
The namespace in this comment is incorrect after refactoring. It should be updated to reflect the new common namespace.
Suggested change
| } // namespace shammodels::sph::modules | |
| } // namespace shammodels::common::modules |
Contributor
Workflow reportworkflow report corresponding to commit d8f535d Pre-commit check reportSome failures were detected in base source checks checks. ❌ Check license headersThe pre-commit checks have found some missing or ill formed license header. Any line break before this header or change in its formatting will trigger the fail of this test
❌ Check doxygen headersThe pre-commit checks have found some files without or with a wrong doxygen file header A properly formed doxygen header should be like and placed just below This test is triggered if the doxygen header is missing or with the wrong file name At some point we will refer to a guide in the doc about this List of files with errors :
❌ end-of-file-fixer❌ trailing-whitespaceSuggested changesDetailed changes :diff --git a/src/shammodels/common/include/shammodels/common/modules/render/CartesianRender.hpp b/src/shammodels/common/include/shammodels/common/modules/render/CartesianRender.hpp
index 843330fd..bb97abd2 100644
--- a/src/shammodels/common/include/shammodels/common/modules/render/CartesianRender.hpp
+++ b/src/shammodels/common/include/shammodels/common/modules/render/CartesianRender.hpp
@@ -34,14 +34,12 @@ namespace shammodels::common::modules {
using Kernel = SPHKernel<Tscal>;
using RenderConfig = shammodels::common::RenderConfig<Tscal>;
- using Storage = TStorage;//SolverStorage<Tvec, u32>;
+ using Storage = TStorage; // SolverStorage<Tvec, u32>;
ShamrockCtx &context;
RenderConfig &render_config;
Storage &storage;
-
-
CartesianRender(ShamrockCtx &context, RenderConfig &render_config, Storage &storage)
: context(context), render_config(render_config), storage(storage) {}
@@ -150,4 +148,4 @@ namespace shammodels::common::modules {
inline PatchScheduler &scheduler() { return shambase::get_check_ref(context.sched); }
};
-} // namespace shammodels::sph::modules
+} // namespace shammodels::common::modules
diff --git a/src/shammodels/common/include/shammodels/common/modules/render/RenderConfig.hpp b/src/shammodels/common/include/shammodels/common/modules/render/RenderConfig.hpp
index 990cea4a..827556e5 100644
--- a/src/shammodels/common/include/shammodels/common/modules/render/RenderConfig.hpp
+++ b/src/shammodels/common/include/shammodels/common/modules/render/RenderConfig.hpp
@@ -17,18 +17,16 @@
*
*/
-
namespace shammodels::common {
- template<class Tscal>
- struct RenderConfig;
-
- template<class Tscal>
- struct RenderConfig {
- //Tscal hfact;
+ template<class Tscal>
+ struct RenderConfig;
+
+ template<class Tscal>
+ struct RenderConfig {
+ // Tscal hfact;
Tscal gpart_mass;
unsigned int tree_reduction_level;
-
};
-}
\ No newline at end of file
+} // namespace shammodels::common
diff --git a/src/shammodels/common/include/shammodels/common/modules/render/RenderFieldGetter.hpp b/src/shammodels/common/include/shammodels/common/modules/render/RenderFieldGetter.hpp
index ffacf78c..493b5eaf 100644
--- a/src/shammodels/common/include/shammodels/common/modules/render/RenderFieldGetter.hpp
+++ b/src/shammodels/common/include/shammodels/common/modules/render/RenderFieldGetter.hpp
@@ -19,8 +19,8 @@
#include "shambackends/DeviceBuffer.hpp"
#include "shambackends/typeAliasVec.hpp"
#include "shambackends/vec.hpp"
-#include "shammodels/common/modules/render/RenderConfig.hpp"
#include "shammath/sphkernels.hpp"
+#include "shammodels/common/modules/render/RenderConfig.hpp"
#include "shampylib/PatchDataToPy.hpp"
#include "shamrock/scheduler/ShamrockCtx.hpp"
#include <pybind11/pytypes.h>
@@ -34,8 +34,8 @@ namespace shammodels::common::modules {
static constexpr u32 dim = shambase::VectorProperties<Tvec>::dimension;
using Kernel = SPHKernel<Tscal>;
- using RenderConfig = common::RenderConfig<Tscal>;
- using Storage = TStorage;//SolverStorage<Tvec, u32>;
+ using RenderConfig = common::RenderConfig<Tscal>;
+ using Storage = TStorage; // SolverStorage<Tvec, u32>;
ShamrockCtx &context;
RenderConfig &render_config;
@@ -60,4 +60,4 @@ namespace shammodels::common::modules {
inline PatchScheduler &scheduler() { return shambase::get_check_ref(context.sched); }
};
-} // namespace shammodels::sph::modules
+} // namespace shammodels::common::modules
diff --git a/src/shammodels/common/src/modules/render/CartesianRender.cpp b/src/shammodels/common/src/modules/render/CartesianRender.cpp
index 420cd34a..b9ec064d 100644
--- a/src/shammodels/common/src/modules/render/CartesianRender.cpp
+++ b/src/shammodels/common/src/modules/render/CartesianRender.cpp
@@ -20,11 +20,11 @@
#include "shammath/AABB.hpp"
#include "shammodels/common/density.hpp"
#include "shammodels/common/modules/render/CartesianRender.hpp"
+#include "shammodels/common/modules/render/RenderFieldGetter.hpp"
+#include "shamrock/scheduler/SchedulerUtility.hpp"
#include "shamtree/KarrasRadixTreeField.hpp"
#include "shamtree/RadixTree.hpp"
#include "shamtree/TreeTraversal.hpp"
-#include "shammodels/common/modules/render/RenderFieldGetter.hpp"
-#include "shamrock/scheduler/SchedulerUtility.hpp"
namespace shammodels::common::modules {
@@ -117,13 +117,14 @@ namespace shammodels::common::modules {
shambase::Timer t;
t.start();
- auto ret = RenderFieldGetter<Tvec, Tfield, SPHKernel, TStorage>(context, render_config, storage)
- .runner_function(
- field_name,
- [&](auto field_getter) -> sham::DeviceBuffer<Tfield> {
- return compute_slice(field_getter, positions);
- },
- custom_getter);
+ auto ret
+ = RenderFieldGetter<Tvec, Tfield, SPHKernel, TStorage>(context, render_config, storage)
+ .runner_function(
+ field_name,
+ [&](auto field_getter) -> sham::DeviceBuffer<Tfield> {
+ return compute_slice(field_getter, positions);
+ },
+ custom_getter);
t.end();
if (shamcomm::world_rank() == 0) {
@@ -154,13 +155,14 @@ namespace shammodels::common::modules {
shambase::Timer t;
t.start();
- auto ret = RenderFieldGetter<Tvec, Tfield, SPHKernel, TStorage>(context, render_config, storage)
- .runner_function(
- field_name,
- [&](auto field_getter) -> sham::DeviceBuffer<Tfield> {
- return compute_column_integ(field_getter, rays);
- },
- custom_getter);
+ auto ret
+ = RenderFieldGetter<Tvec, Tfield, SPHKernel, TStorage>(context, render_config, storage)
+ .runner_function(
+ field_name,
+ [&](auto field_getter) -> sham::DeviceBuffer<Tfield> {
+ return compute_column_integ(field_getter, rays);
+ },
+ custom_getter);
t.end();
if (shamcomm::world_rank() == 0) {
@@ -191,13 +193,14 @@ namespace shammodels::common::modules {
shambase::Timer t;
t.start();
- auto ret = RenderFieldGetter<Tvec, Tfield, SPHKernel, TStorage>(context, render_config, storage)
- .runner_function(
- field_name,
- [&](auto field_getter) -> sham::DeviceBuffer<Tfield> {
- return compute_azymuthal_integ(field_getter, ring_rays);
- },
- custom_getter);
+ auto ret
+ = RenderFieldGetter<Tvec, Tfield, SPHKernel, TStorage>(context, render_config, storage)
+ .runner_function(
+ field_name,
+ [&](auto field_getter) -> sham::DeviceBuffer<Tfield> {
+ return compute_azymuthal_integ(field_getter, ring_rays);
+ },
+ custom_getter);
t.end();
if (shamcomm::world_rank() == 0) {
@@ -619,4 +622,4 @@ namespace shammodels::common::modules {
return compute_column_integ(field_name, rays, custom_getter);
}
-} // namespace shammodels::sph::modules
+} // namespace shammodels::common::modules
diff --git a/src/shammodels/common/src/modules/render/RenderFieldGetter.cpp b/src/shammodels/common/src/modules/render/RenderFieldGetter.cpp
index 8ce79755..46e5c5f0 100644
--- a/src/shammodels/common/src/modules/render/RenderFieldGetter.cpp
+++ b/src/shammodels/common/src/modules/render/RenderFieldGetter.cpp
@@ -100,7 +100,7 @@ namespace shammodels::common::modules {
auto e = q.submit(depends_list, [&](sycl::handler &cgh) {
cgh.parallel_for(
sycl::range<1>{pdat.get_obj_cnt()}, [=](sycl::item<1> item) {
- u32 gid = (u32) item.get_id();
+ u32 gid = (u32) item.get_id();
acc_inv_hpart[gid] = 1.0 / acc_h[gid];
});
});
@@ -193,4 +193,4 @@ namespace shammodels::common::modules {
return lambda(field_source_getter);
}
-} // namespace shammodels::sph::modules
+} // namespace shammodels::common::modules
diff --git a/src/shammodels/gsph/include/shammodels/gsph/Model.hpp b/src/shammodels/gsph/include/shammodels/gsph/Model.hpp
index c2901d11..b63d13cf 100644
--- a/src/shammodels/gsph/include/shammodels/gsph/Model.hpp
+++ b/src/shammodels/gsph/include/shammodels/gsph/Model.hpp
@@ -33,9 +33,9 @@
#include "shambackends/vec.hpp"
#include "shamcomm/collectives.hpp"
#include "shamcomm/logs.hpp"
+#include "shammodels/common/density.hpp"
#include "shammodels/common/setup/generators.hpp"
#include "shammodels/gsph/Solver.hpp"
-#include "shammodels/common/density.hpp"
#include "shamrock/io/ShamrockDump.hpp"
#include "shamrock/patch/PatchDataLayer.hpp"
#include "shamrock/scheduler/ReattributeDataUtility.hpp"
diff --git a/src/shammodels/gsph/src/modules/UpdateDerivs.cpp b/src/shammodels/gsph/src/modules/UpdateDerivs.cpp
index 48772a37..ea17f2fe 100644
--- a/src/shammodels/gsph/src/modules/UpdateDerivs.cpp
+++ b/src/shammodels/gsph/src/modules/UpdateDerivs.cpp
@@ -29,10 +29,10 @@
#include "shammodels/gsph/modules/UpdateDerivs.hpp"
#include "shambackends/math.hpp"
#include "shammath/sphkernels.hpp"
+#include "shammodels/common/density.hpp"
#include "shammodels/gsph/config/FieldNames.hpp"
#include "shammodels/gsph/math/forces.hpp"
#include "shammodels/gsph/math/riemann/iterative.hpp"
-#include "shammodels/common/density.hpp"
#include "shamsys/legacy/log.hpp"
#include "shamtree/TreeTraversal.hpp"
diff --git a/src/shammodels/gsph/src/modules/render/RenderInstanciations.cpp b/src/shammodels/gsph/src/modules/render/RenderInstanciations.cpp
index b528d33a..de7e914c 100644
--- a/src/shammodels/gsph/src/modules/render/RenderInstanciations.cpp
+++ b/src/shammodels/gsph/src/modules/render/RenderInstanciations.cpp
@@ -1,12 +1,12 @@
#include "shambase/aliases_float.hpp"
-#include "shammodels/common/modules/render/RenderFieldGetter.hpp"
-#include "shammodels/common/modules/render/CartesianRender.hpp"
-#include "shammodels/gsph/modules/SolverStorage.hpp"
#include "shambackends/DeviceBuffer.hpp"
#include "shambackends/typeAliasVec.hpp"
#include "shambackends/vec.hpp"
-#include "shammodels/common/modules/render/RenderConfig.hpp"
#include "shammath/sphkernels.hpp"
+#include "shammodels/common/modules/render/CartesianRender.hpp"
+#include "shammodels/common/modules/render/RenderConfig.hpp"
+#include "shammodels/common/modules/render/RenderFieldGetter.hpp"
+#include "shammodels/gsph/modules/SolverStorage.hpp"
#include "shampylib/PatchDataToPy.hpp"
#include "shamrock/scheduler/ShamrockCtx.hpp"
#include <pybind11/pytypes.h>
@@ -15,35 +15,59 @@
using namespace shammath;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
-//gsph
-template class shammodels::common::modules::CartesianRender<f64_3, f64, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
+// gsph
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64_3, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64_3, M4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64_3, M6, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64_3, M8, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64_3, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
\ No newline at end of file
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64_3, C2, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64_3, C4, shammodels::gsph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ CartesianRender<f64_3, f64_3, C6, shammodels::gsph::SolverStorage<f64_3, u32>>;
diff --git a/src/shammodels/gsph/src/pyGSPHModel.cpp b/src/shammodels/gsph/src/pyGSPHModel.cpp
index aab49b20..2bdee64b 100644
--- a/src/shammodels/gsph/src/pyGSPHModel.cpp
+++ b/src/shammodels/gsph/src/pyGSPHModel.cpp
@@ -29,9 +29,10 @@
#include "shambindings/pytypealias.hpp"
#include "shamcomm/worldInfo.hpp"
#include "shammath/sphkernels.hpp"
+#include "shammodels/common/modules/render/CartesianRender.hpp"
+#include "shammodels/common/modules/render/RenderConfig.hpp"
#include "shammodels/common/shamrock_json_to_py_json.hpp"
#include "shammodels/gsph/Model.hpp"
-#include "shamrock/scheduler/PatchScheduler.hpp"
#include "shammodels/sph/Model.hpp"
#include "shammodels/sph/io/PhantomDump.hpp"
#include "shammodels/sph/modules/AnalysisBarycenter.hpp"
@@ -40,9 +41,7 @@
#include "shammodels/sph/modules/AnalysisEnergyPotential.hpp"
#include "shammodels/sph/modules/AnalysisSodTube.hpp"
#include "shammodels/sph/modules/AnalysisTotalMomentum.hpp"
-#include "shammodels/common/modules/render/CartesianRender.hpp"
-#include "shammodels/common/modules/render/RenderConfig.hpp"
-
+#include "shamrock/scheduler/PatchScheduler.hpp"
#include <pybind11/cast.h>
#include <pybind11/numpy.h>
#include <memory>
@@ -53,10 +52,10 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
using Tscal = shambase::VecComponent<Tvec>;
- using T = Model<Tvec, SPHKernel>;
- using TConfig = typename T::SolverConfig;
+ using T = Model<Tvec, SPHKernel>;
+ using TConfig = typename T::SolverConfig;
using custom_getter_t = std::function<pybind11::array_t<f64>(size_t, pybind11::dict &)>;
- using RenderConfig = shammodels::common::RenderConfig<Tscal>;
+ using RenderConfig = shammodels::common::RenderConfig<Tscal>;
shamlog_debug_ln("[Py]", "registering class :", name_config, typeid(T).name());
shamlog_debug_ln("[Py]", "registering class :", name_model, typeid(T).name());
@@ -424,7 +423,7 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
-------
>>> model.dump("checkpoint.shamrock")
)==")
-.def(
+ .def(
"render_cartesian_slice",
[](T &self,
const std::string &name,
@@ -436,11 +435,9 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
u32 ny,
const std::optional<custom_getter_t> &custom_getter)
-> std::variant<py::array_t<Tscal>> {
-
- shammodels::common::RenderConfig<Tscal> rc{
- self.solver.solver_config.gpart_mass,
- self.solver.solver_config.tree_reduction_level
- };
+ shammodels::common::RenderConfig<Tscal> rc{
+ self.solver.solver_config.gpart_mass,
+ self.solver.solver_config.tree_reduction_level};
if (custom_getter.has_value()) {
if (!(name == "custom" && field_type == "f64")) {
throw shambase::make_except_with_loc<std::invalid_argument>(
@@ -451,8 +448,12 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
if (field_type == "f64") {
py::array_t<Tscal> ret({ny, nx});
- shammodels::common::modules::CartesianRender<Tvec, f64, SPHKernel, shammodels::gsph::SolverStorage<Tvec, u32>> render(
- self.ctx, rc, self.solver.storage);
+ shammodels::common::modules::CartesianRender<
+ Tvec,
+ f64,
+ SPHKernel,
+ shammodels::gsph::SolverStorage<Tvec, u32>>
+ render(self.ctx, rc, self.solver.storage);
std::vector<f64> slice
= render
@@ -471,10 +472,12 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
if (field_type == "f64_3") {
py::array_t<Tscal> ret({ny, nx, 3_u32});
- shammodels::common::modules::CartesianRender<Tvec, f64_3, SPHKernel, shammodels::gsph::SolverStorage<Tvec, u32>> render(
- self.ctx,
- rc,
- self.solver.storage);
+ shammodels::common::modules::CartesianRender<
+ Tvec,
+ f64_3,
+ SPHKernel,
+ shammodels::gsph::SolverStorage<Tvec, u32>>
+ render(self.ctx, rc, self.solver.storage);
std::vector<f64_3> slice
= render.compute_slice(name, center, delta_x, delta_y, nx, ny, std::nullopt)
@@ -514,10 +517,9 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
u32 ny,
const std::optional<custom_getter_t> &custom_getter)
-> std::variant<py::array_t<Tscal>> {
- shammodels::common::RenderConfig<Tscal> rc{
- self.solver.solver_config.gpart_mass,
- self.solver.solver_config.tree_reduction_level
- };
+ shammodels::common::RenderConfig<Tscal> rc{
+ self.solver.solver_config.gpart_mass,
+ self.solver.solver_config.tree_reduction_level};
if (custom_getter.has_value()) {
if (!(name == "custom" && field_type == "f64")) {
throw shambase::make_except_with_loc<std::invalid_argument>(
@@ -528,8 +530,12 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
if (field_type == "f64") {
py::array_t<Tscal> ret({ny, nx});
- shammodels::common::modules::CartesianRender<Tvec, f64, SPHKernel, shammodels::gsph::SolverStorage<Tvec, u32>> render(
- self.ctx, rc, self.solver.storage);
+ shammodels::common::modules::CartesianRender<
+ Tvec,
+ f64,
+ SPHKernel,
+ shammodels::gsph::SolverStorage<Tvec, u32>>
+ render(self.ctx, rc, self.solver.storage);
std::vector<f64> slice
= render
@@ -549,8 +555,12 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
if (field_type == "f64_3") {
py::array_t<Tscal> ret({ny, nx, 3_u32});
- shammodels::common::modules::CartesianRender<Tvec, f64_3, SPHKernel, shammodels::gsph::SolverStorage<Tvec, u32>> render(
- self.ctx, rc, self.solver.storage);
+ shammodels::common::modules::CartesianRender<
+ Tvec,
+ f64_3,
+ SPHKernel,
+ shammodels::gsph::SolverStorage<Tvec, u32>>
+ render(self.ctx, rc, self.solver.storage);
std::vector<f64_3> slice
= render
@@ -581,7 +591,7 @@ void add_gsph_instance(py::module &m, std::string name_config, std::string name_
py::arg("ny"),
py::arg("custom_getter") = std::nullopt)
-;
+ ;
}
using namespace shammodels::gsph;
diff --git a/src/shammodels/sph/include/shammodels/sph/Model.hpp b/src/shammodels/sph/include/shammodels/sph/Model.hpp
index 6610f27f..c40b023d 100644
--- a/src/shammodels/sph/include/shammodels/sph/Model.hpp
+++ b/src/shammodels/sph/include/shammodels/sph/Model.hpp
@@ -25,10 +25,10 @@
#include "shambackends/vec.hpp"
#include "shamcomm/collectives.hpp"
#include "shamcomm/logs.hpp"
+#include "shammodels/common/density.hpp"
#include "shammodels/common/setup/generators.hpp"
#include "shammodels/sph/Solver.hpp"
#include "shammodels/sph/io/PhantomDump.hpp"
-#include "shammodels/common/density.hpp"
#include "shammodels/sph/modules/ComputeLoadBalanceValue.hpp"
#include "shammodels/sph/modules/SPHSetup.hpp"
#include "shamrock/io/ShamrockDump.hpp"
diff --git a/src/shammodels/sph/include/shammodels/sph/SPHUtilities.hpp b/src/shammodels/sph/include/shammodels/sph/SPHUtilities.hpp
index 77777526..4f92ecc8 100644
--- a/src/shammodels/sph/include/shammodels/sph/SPHUtilities.hpp
+++ b/src/shammodels/sph/include/shammodels/sph/SPHUtilities.hpp
@@ -16,8 +16,8 @@
*
*/
-#include "shammodels/sph/BasicSPHGhosts.hpp"
#include "shammodels/common/density.hpp"
+#include "shammodels/sph/BasicSPHGhosts.hpp"
#include "shamrock/scheduler/PatchScheduler.hpp"
#include "shamtree/RadixTree.hpp"
#include "shamtree/TreeTraversal.hpp"
diff --git a/src/shammodels/sph/src/Solver.cpp b/src/shammodels/sph/src/Solver.cpp
index dd480642..93c462c6 100644
--- a/src/shammodels/sph/src/Solver.cpp
+++ b/src/shammodels/sph/src/Solver.cpp
@@ -33,6 +33,7 @@
#include "shamcomm/worldInfo.hpp"
#include "shamcomm/wrapper.hpp"
#include "shammath/sphkernels.hpp"
+#include "shammodels/common/density.hpp"
#include "shammodels/common/modules/ForwardEuler.hpp"
#include "shammodels/common/timestep_report.hpp"
#include "shammodels/sph/BasicSPHGhosts.hpp"
@@ -40,7 +41,6 @@
#include "shammodels/sph/Solver.hpp"
#include "shammodels/sph/SolverConfig.hpp"
#include "shammodels/sph/io/PhantomDump.hpp"
-#include "shammodels/common/density.hpp"
#include "shammodels/sph/math/forces.hpp"
#include "shammodels/sph/math/q_ab.hpp"
#include "shammodels/sph/modules/BuildTrees.hpp"
diff --git a/src/shammodels/sph/src/modules/io/VTKDump.cpp b/src/shammodels/sph/src/modules/io/VTKDump.cpp
index 186fb717..3abf5bc1 100644
--- a/src/shammodels/sph/src/modules/io/VTKDump.cpp
+++ b/src/shammodels/sph/src/modules/io/VTKDump.cpp
@@ -18,8 +18,8 @@
#include "shammodels/sph/modules/io/VTKDump.hpp"
#include "shambackends/kernel_call.hpp"
-#include "shammodels/common/io/VTKDumpUtils.hpp"
#include "shammodels/common/density.hpp"
+#include "shammodels/common/io/VTKDumpUtils.hpp"
#include "shamrock/io/LegacyVtkWritter.hpp"
#include "shamrock/patch/PatchDataFieldSpan.hpp"
#include "shamrock/scheduler/SchedulerUtility.hpp"
diff --git a/src/shammodels/sph/src/modules/render/RenderInstanciations.cpp b/src/shammodels/sph/src/modules/render/RenderInstanciations.cpp
index ffe7dc60..6fee193a 100644
--- a/src/shammodels/sph/src/modules/render/RenderInstanciations.cpp
+++ b/src/shammodels/sph/src/modules/render/RenderInstanciations.cpp
@@ -1,12 +1,12 @@
#include "shambase/aliases_float.hpp"
-#include "shammodels/common/modules/render/RenderFieldGetter.hpp"
-#include "shammodels/common/modules/render/CartesianRender.hpp"
-#include "shammodels/sph/modules/SolverStorage.hpp"
#include "shambackends/DeviceBuffer.hpp"
#include "shambackends/typeAliasVec.hpp"
#include "shambackends/vec.hpp"
-#include "shammodels/common/modules/render/RenderConfig.hpp"
#include "shammath/sphkernels.hpp"
+#include "shammodels/common/modules/render/CartesianRender.hpp"
+#include "shammodels/common/modules/render/RenderConfig.hpp"
+#include "shammodels/common/modules/render/RenderFieldGetter.hpp"
+#include "shammodels/sph/modules/SolverStorage.hpp"
#include "shampylib/PatchDataToPy.hpp"
#include "shamrock/scheduler/ShamrockCtx.hpp"
#include <pybind11/pytypes.h>
@@ -15,35 +15,58 @@
using namespace shammath;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, M4, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, M6, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, M8, shammodels::sph::SolverStorage<f64_3, u32>>;
-
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, C2, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, C4, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64, C6, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, M4, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, M6, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, M8, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, M4, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, M6, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, M8, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, C2, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, C4, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64, C6, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, C2, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, C4, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::RenderFieldGetter<f64_3, f64_3, C6, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, M4, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, M6, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, M8, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, M4, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, M6, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, M8, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, C2, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, C4, shammodels::sph::SolverStorage<f64_3, u32>>;
+template class shammodels::common::modules::
+ RenderFieldGetter<f64_3, f64_3, C6, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common::modules::CartesianRender<f64_3, f64, C2, shammodels::sph::SolverStorage<f64_3, u32>>;
-template class shammodels::common
... truncated ...
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Essentially what I did is:
Current issue: I need to move the template instantiations elsewhere (not in RenderXXX.cpp), otherwise there is a circular dependency.
@ gemini I don't want your feedback on this yet, I'm opening this draft PR to share it with human colleagues to have their opinion on the general structure.