[SPH] add dust positivity limiter#1908
Draft
tdavidcl wants to merge 30 commits into
Draft
Conversation
for more information, see https://pre-commit.ci
![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 introduces a new ComputeCFLDust1Fluid module to calculate CFL conditions for dust in a 1-fluid model, registers the Ts_j field in the solver, and updates the run_dustysettle_tvi.py example to use periodic HCP box calculations and compute L2 errors. Feedback on these changes highlights a critical compilation error in ComputeCFLDust1Fluid.hpp caused by variable shadowing of Ts_j, commented-out CFL dust computation code in Solver.cpp that should be enabled, and an unprofessional string literal exception. Additionally, there are several instances of leftover debugging code and print statements in NodeUpdateDerivsMonofluidTVI.cpp and run_dustysettle_tvi.py that should be removed before merging.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
I am having trouble creating individual review comments. Click here to see my feedback.
src/shammodels/sph/include/shammodels/sph/modules/ComputeCFLDust1Fluid.hpp (107-108)
The local variable Ts_j shadows the parameter pointer Ts_j of the lambda. This leads to a compilation error because Ts_j on the right-hand side of the assignment is resolved to the local variable itself (which is a scalar Tscal and cannot be subscripted). Rename the local variable to avoid shadowing and fix the compilation error.
Tscal Ts_val = Ts_j[id_a_d + j];
Tscal Ts2_j = Ts_val * Ts_val;
src/shammodels/sph/src/Solver.cpp (2703-2712)
The code to instantiate and set up ComputeCFLDust1Fluid is commented out, meaning the dust CFL condition is not being computed or applied. Furthermore, the commented-out code references s_j_refs and Ts_j_refs which are not defined in this scope. To enable this feature, uncomment the block, retrieve the edges from the solver graph, and evaluate the node.
std::shared_ptr<ComputeCFLDust1Fluid<Tscal>> compute_cfl_dust1_fluid;
if (solver_config.dust_config.has_s_j_field()) {
u32 ndust = solver_config.dust_config.get_dust_nvar();
compute_cfl_dust1_fluid = std::make_shared<ComputeCFLDust1Fluid<Tscal>>(ndust);
auto s_j_refs = storage.solver_graph.template get_edge_ptr<shamrock::solvergraph::FieldRefs<Tscal>>("s_j");
auto Ts_j_refs = storage.solver_graph.template get_edge_ptr<shamrock::solvergraph::Field<Tscal>>("Ts_j");
compute_cfl_dust1_fluid->set_edges(
storage.part_counts, C_cour_edge, hpart_refs, storage.soundspeed, s_j_refs,
Ts_j_refs, cfl_dt);
}
src/shammodels/sph/src/Solver.cpp (1595-1599)
Throwing a string literal is bad practice in C++ and the message is unprofessional. It should throw a standard exception class like std::runtime_error with a descriptive message.
if (std::holds_alternative<None>(cfg.dust_drag_mode)) {
throw std::runtime_error("Dust drag mode 'None' is not supported when s_j field is enabled.");
}src/shammodels/sph/src/modules/NodeUpdateDerivsMonofluidTVI.cpp (102-111)
This is leftover debugging code with hardcoded particle/dust indices and printf statements. It should be removed from the production codebase.
src/shammodels/sph/src/modules/NodeUpdateDerivsMonofluidTVI.cpp (126-144)
These blocks are leftover debugging code with hardcoded conditions and printf statements. They should be removed from the production codebase.
examples/sph/run_dustysettle_tvi.py (773-774)
Setting reference_dusty_settle = None immediately after instantiating it disables the reference model and the subsequent L2 error analysis. This appears to be leftover debugging code and should be removed.
reference_dusty_settle = ReferenceDustySettleAll()
Contributor
|
Thanks @tdavidcl 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: |
tdavidcl
commented
Jul 13, 2026
tdavidcl
commented
Jul 14, 2026
tdavidcl
commented
Jul 14, 2026
tdavidcl
commented
Jul 14, 2026
tdavidcl
changed the title
Contributor
Workflow reportworkflow report corresponding to commit 6cb6d77 Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Clang-tidy diff reportDoxygen diff with
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
No description provided.