fix master copy before refinement + tensorfield and refluxing#1068
fix master copy before refinement + tensorfield and refluxing#1068UCaromel wants to merge 8 commits intoPHAREHUB:masterfrom
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThis update introduces comprehensive support for tensor field data and operations in the AMR framework, including new tensor field data structures, factories, geometry, overlap, and variable classes. It adds tensor-aware coarsen/refine/time-interpolate operators, modifies electric/magnetic field refinement and coarsening, and refactors messenger, solver, and resource management interfaces to support tensor and vector fields. Numerous test and utility updates reflect these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AMRFramework
participant TensorFieldData
participant TensorFieldRefineOperator
participant TensorFieldCoarsenOperator
participant TensorFieldLinearTimeInterpolate
participant Messenger
participant Solver
User->>AMRFramework: triggers AMR step (advance, refine, coarsen, synchronize)
AMRFramework->>TensorFieldRefineOperator: refine tensor field on patches
TensorFieldRefineOperator->>TensorFieldData: component-wise refinement
AMRFramework->>TensorFieldCoarsenOperator: coarsen tensor field on patches
TensorFieldCoarsenOperator->>TensorFieldData: component-wise coarsening
AMRFramework->>TensorFieldLinearTimeInterpolate: time interpolate tensor field
TensorFieldLinearTimeInterpolate->>TensorFieldData: component-wise interpolation
AMRFramework->>Messenger: synchronize/interpolate ghost zones
Messenger->>TensorFieldData: fill ghost/interior using fill patterns
AMRFramework->>Solver: advance level, accumulate/reset flux sums, reflux
Solver->>TensorFieldData: update, accumulate, and apply fluxes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
src/amr/data/field/coarsening/electric_field_coarsener.hpp (1)
18-31: Update documentation to reflect electric field coarsening.The class documentation still references
MagneticFieldCoarsenerand describes magnetic flux conservation, but this class now handles electric field coarsening.Update the documentation to accurately describe the electric field coarsening operation and its purpose.
♻️ Duplicate comments (1)
src/amr/data/field/field_variable_fill_pattern.hpp (1)
99-99: Remove commented codeThis line should be removed as previously requested.
🧹 Nitpick comments (27)
tests/simulator/test_initialization.py (1)
355-362: Remove leftover debug
print("fpx", fpx)was helpful while debugging but will spam stdout every time the test-suite runs. Preferself.logger.debug,pytest-capture, or simply delete the line.- print("fpx", fpx)tests/amr/data/field/refine/CMakeLists.txt (1)
47-49: Add TODO when disabling testsCommenting out tests tends to become permanent tech-debt. Please add a clear TODO or issue reference and, if possible, move the test to
add_disabled_test()so it is still compiled and reported as skipped.-# _add_general_amr_field_refine_test(test_field_refinement_on_hierarchy) -# _add_serial_amr_field_refine_test(test_field_refine) +# TODO(##issue-id##): re-enable once registerRefine supports multiple quantities +# _add_general_amr_field_refine_test(test_field_refinement_on_hierarchy) +# _add_serial_amr_field_refine_test(test_field_refine)tests/core/numerics/interpolator/test_main.cpp (1)
534-537: Use direct member calls instead of pointer gymnastics
(*(&rho)).zero();works but is unnecessarily convoluted. Direct calls are clearer and generate identical code.- (*(&rho)).zero(); - (*(&rho_c)).zero(); - v.zero(); + rho.zero(); + rho_c.zero(); + v.zero();Also applies to: 710-713
tests/simulator/advance/test_fields_advance_1d.py (1)
68-69: Clarify the redundant arithmetic operation.The
+ 0operations appear redundant. If this is intentional for future modifications or clarity, consider adding a comment. Otherwise, simplify to just usesmallest_patch_sizedirectly.- smallest_patch_size=smallest_patch_size + 0, - largest_patch_size=smallest_patch_size + 0, + smallest_patch_size=smallest_patch_size, + largest_patch_size=smallest_patch_size,tests/functional/harris/harris_2d.py (1)
3-3: Remove unused import.The
osmodule is imported but never used in the code.-import ossrc/core/data/ndarray/ndarray_vector.hpp (1)
243-243: Consider using lowercase for parameter namesTemplate parameter packs typically use lowercase names for consistency with standard library conventions.
- template<FloatingPoint U = DataType, typename... Nodes> - explicit NdArrayVector(Nodes... nodes) + template<FloatingPoint U = DataType, typename... nodes> + explicit NdArrayVector(nodes... n)Also applies to: 257-257
src/amr/resources_manager/resources_manager_utilities.hpp (1)
101-101: Improve error messages for better debuggingThe "bad condition" error messages are not descriptive. Consider providing more context about what went wrong.
- throw std::runtime_error("bad condition"); + throw std::runtime_error("ResourceResolver: Unknown resource type - not a field, tensor field, or particles");Also applies to: 118-118
src/amr/data/tensorfield/tensor_field_overlap.hpp (2)
77-85: Unused static method _get_index_for.This method for mapping Component enums to indices is not used within the class. Consider removing it if not needed elsewhere.
86-94: Transformation comparison limited to non-rotated cases.The
transformations_equal_method only accepts transformations with NO_ROTATE. Consider documenting this limitation or extending to support rotated transformations if needed.tests/simulator/test_advance.py (2)
77-77: Restore diagnostic directory cleanup after debugging.The cleanup registration is commented out, which will cause test output directories to accumulate. Please restore this once debugging is complete.
402-404: Restore exception re-raising after debugging.The exception re-raising is commented out, which prevents tests from properly failing. Please restore this behavior once debugging is complete.
-# if self.rethrow_: -# raise e +if self.rethrow_: + raise esrc/amr/messengers/messenger.hpp (1)
13-13: Remove commented-out include statementThis commented-out include appears to be dead code and should be removed for cleaner code maintenance.
-// #include "core/data/grid/gridlayout.hpp"src/amr/messengers/hybrid_messenger_info.hpp (1)
38-38: Remove commented-out using statementThis commented-out statement appears to be dead code and should be removed.
- // using std::string = core::std::string;src/amr/messengers/refiner.hpp (1)
34-36: Consider making tensor rank configurableThe tensor rank is hard-coded to 2 with a comment indicating "there's no real tensorfields that use this code yet". This might limit future extensibility when tensor fields of different ranks are needed.
Consider making the tensor rank a template parameter or configuration option to support tensor fields of various ranks in the future.
src/amr/data/field/coarsening/electric_field_coarsener.hpp (1)
154-154: Update the endif comment to match the header guard.-#endif +#endif // PHARE_ELECTRIC_FIELD_COARSENER_HPPsrc/amr/data/field/time_interpolate/field_linear_time_interpolate.hpp (2)
32-32: Consider using explicit types for better readability.The structured binding uses
autowhich makes it unclear what types are being unpacked. Consider being more explicit about the expected types for better code clarity.
92-92: Remove empty comment.- //src/amr/data/tensorfield/tensor_field_geometry.hpp (4)
37-37: Use default destructor.- virtual ~TensorFieldGeometryBase() {} + virtual ~TensorFieldGeometryBase() = default;
38-40: Address the TODO comment about validation.The constructor has a comment suggesting validation of patch boxes across components, but this check is not implemented.
Would you like me to implement the patch box validation to ensure all components have the same patch box?
153-154: Use explicit type instead of auto for constructor parameter.Using
autofor constructor parameters reduces code clarity. Consider using the explicit type.- TensorFieldGeometry(SAMRAI::hier::Box const& box, GridLayoutT const& layout, tensor_t const qty, - auto geoms) + TensorFieldGeometry(SAMRAI::hier::Box const& box, GridLayoutT const& layout, tensor_t const qty, + std::pair<std::array<std::shared_ptr<FieldGeometryBase<GridLayoutT::dimension>>, N>, + std::array<std::shared_ptr<FieldGeometry_t>, N>> geoms)
158-164: Improve error message with component index.The error message could be more descriptive by including which component is null.
- for (auto component : components_) + for (std::size_t i = 0; i < components_.size(); ++i) { - if (!component) + if (!components_[i]) { - throw std::runtime_error("TensorFieldGeometry: component is null"); + throw std::runtime_error("TensorFieldGeometry: component " + std::to_string(i) + " is null"); } }src/amr/solvers/solver_ppc.hpp (1)
296-296: Consider using explicit type for layout variable.Using
autofor the layout variable reduces code clarity. Consider using the explicit typeGridLayoutfor better readability.For example, at line 296:
- auto const& layout = amr::layoutFromPatch<GridLayout>(*patch); + GridLayout const& layout = amr::layoutFromPatch<GridLayout>(*patch);Also applies to: 324-324, 343-343
src/amr/data/field/refine/field_refine_operator.hpp (1)
30-31: Consider using explicit template parameters for better clarity.While using
autoparameters is valid in C++20, consider making the template parameters more explicit for better documentation and type safety:-template<typename Dst> -void refine_field(Dst& destinationField, auto& sourceField, auto& intersectionBox, auto& refiner) +template<typename Dst, typename Src, typename Box, typename Refiner> +void refine_field(Dst& destinationField, Src& sourceField, Box& intersectionBox, Refiner& refiner)src/amr/data/field/coarsening/field_coarsen_operator.hpp (2)
23-24: Consider using explicit template parameters for consistency.Similar to
refine_field, consider making the template parameters more explicit:-template<typename Dst> -void coarsen_field(Dst& destinationField, auto& sourceField, auto& intersectionBox, auto& coarsener) +template<typename Dst, typename Src, typename Box, typename Coarsener> +void coarsen_field(Dst& destinationField, Src& sourceField, Box& intersectionBox, Coarsener& coarsener)
256-257: Fix typo in comment.- * selected. Finnaly loop over the indexes in the box, and apply the coarsening defined + * selected. Finally loop over the indexes in the box, and apply the coarsening definedsrc/amr/data/tensorfield/tensor_field_data_factory.hpp (1)
89-103: Consider using named constants for dummy values.While the comment explains that dl and origin are dummy values, consider using named constants to make this more explicit:
+ static constexpr double DUMMY_DL = 0.01; + static constexpr double DUMMY_ORIGIN = 0.0; + for (std::size_t iDim = 0; iDim < dimension; ++iDim) { - dl[iDim] = 0.01; + dl[iDim] = DUMMY_DL; nbCell[iDim] = box.numberCells(iDim); - origin[iDim] = 0; + origin[iDim] = DUMMY_ORIGIN; }src/amr/data/field/field_variable_fill_pattern.hpp (1)
142-142: Remove unused member variableThe member variable
scalar_fill_pattern_is initialized but never used. Consider removing it to avoid unnecessary object construction.public: TensorFieldFillPattern(bool overwrite_interior = false) - : scalar_fill_pattern_{overwrite_interior} - , overwrite_interior_{overwrite_interior} + : overwrite_interior_{overwrite_interior} { } // ... rest of the class ... private: - FieldFillPattern<dimension> scalar_fill_pattern_; bool overwrite_interior_;Also applies to: 209-209
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (4)
src/amr/data/field/refine/magnetic_field_regrider.hpp (2)
39-39: Unused parameter should be commentedAs noted in the previous review, the
ratioparameter is unused and should be commented out.- SAMRAI::hier::IntVector const& ratio) + SAMRAI::hier::IntVector const& /*ratio*/)
58-60: Variables can be made constAs noted in the previous review, these variables can be const.
- auto locFineIdx = AMRToLocal(fineIndex, fineBox_); - auto coarseIdx = toCoarseIndex(fineIndex); - auto locCoarseIdx = AMRToLocal(coarseIdx, coarseBox_); + auto const locFineIdx = AMRToLocal(fineIndex, fineBox_); + auto const coarseIdx = toCoarseIndex(fineIndex); + auto const locCoarseIdx = AMRToLocal(coarseIdx, coarseBox_);src/amr/data/field/refine/magnetic_refine_patch_strategy.hpp (1)
79-84: Simplify compile-time field box generation.As suggested in the past review comment, this could be simplified using a helper function.
Consider using the suggested helper from the past review to simplify this compile-time loop.
src/amr/data/field/field_variable_fill_pattern.hpp (1)
98-99: Remove the commented-out codeThe commented condition should be removed as it appears to be leftover debugging code.
🧹 Nitpick comments (31)
tests/simulator/test_initialization.py (1)
361-361: Remove debugging print statement.The print statement appears to be debugging code that should be removed before committing. It will clutter the test output without providing value in production.
- print("fpx", fpx)tests/amr/data/field/refine/CMakeLists.txt (1)
47-49: Track the disabled tests to ensure they are re-enabled.The temporary disabling of these tests is understandable given the broken
registerRefinefunctionality. However, ensure this is tracked properly so the tests are re-enabled once the underlying issue is fixed.Consider creating a GitHub issue to track the broken
registerRefinefunctionality and the disabled tests. This will help ensure the tests are re-enabled when the functionality is fixed.tests/core/numerics/interpolator/test_main.cpp (2)
534-536: Simplify the zero initialization syntax.The explicit zero initialization is good for test determinism, but the syntax can be simplified.
- (*(&rho)).zero(); - (*(&rho_c)).zero(); - v.zero(); + rho.zero(); + rho_c.zero(); + v.zero();
710-712: Simplify the zero initialization syntax.Same issue as in the 1D constructor - the syntax can be simplified.
- (*(&rho)).zero(); - (*(&rho_c)).zero(); - v.zero(); + rho.zero(); + rho_c.zero(); + v.zero();tests/simulator/advance/test_fields_advance_1d.py (1)
68-69: Clarify the purpose of adding zero to patch size parameters.The
+ 0additions don't change the values but suggest either a workaround or placeholder. Consider removing them unless they serve a specific purpose.- smallest_patch_size=smallest_patch_size + 0, - largest_patch_size=smallest_patch_size + 0, + smallest_patch_size=smallest_patch_size, + largest_patch_size=smallest_patch_size,tests/functional/harris/harris_2d.py (1)
3-3: Remove unused import.The
osmodule is imported but never used in the code.-import ossrc/amr/messengers/hybrid_messenger_info.hpp (1)
38-38: Remove invalid commented code.This commented line contains invalid C++ syntax. The
usingdirective cannot redefinestd::string.- // using std::string = core::std::string;src/core/data/ndarray/ndarray_vector.hpp (1)
273-273: Remove unnecessary blank line.-src/amr/solvers/solver_mhd.hpp (1)
45-64: Empty stub implementations for reflux methodsAll new reflux-related methods have empty implementations. If MHD solver doesn't require refluxing, consider adding comments explaining why these operations are no-ops. If refluxing will be implemented later, add TODO comments.
void prepareStep(IPhysicalModel<AMR_Types>& model, SAMRAI::hier::PatchLevel& level, double const currentTime) override { + // MHD solver does not require refluxing preparation }src/amr/data/tensorfield/tensor_field_overlap.hpp (1)
86-93: Consider simplifying transformation comparisonThe transformation comparison only checks for no rotation and matching offsets/blocks. If rotation support is never needed, document this limitation.
+ // Note: This comparison assumes transformations without rotation bool transformations_equal_(const SAMRAI::hier::Transformation& a, const SAMRAI::hier::Transformation& b)src/amr/data/field/refine/magnetic_field_regrider.hpp (1)
144-147: Additional const opportunity in 3D caseFor consistency with the suggestion above, these variables in the 3D case should also be const.
- auto ix = locCoarseIdx[dirX]; - auto iy = locCoarseIdx[dirY]; - auto iz = locCoarseIdx[dirZ]; + auto const ix = locCoarseIdx[dirX]; + auto const iy = locCoarseIdx[dirY]; + auto const iz = locCoarseIdx[dirZ];src/amr/messengers/refiner.hpp (1)
35-36: Hard-coded rank assumption needs clarificationThe comment indicates that rank is hard-coded to 2 because "there's no real tensorfields that use this code yet". This creates technical debt and potential confusion.
Consider either:
- Making the rank configurable via a template parameter
- Adding a more detailed TODO comment explaining when this will be addressed
- // hard coded rank cause there's no real tensorfields that use this code yet - using TensorFieldData_t = ResourcesManager::template UserTensorField_t<2>::patch_data_type; + // TODO: Rank is currently hard-coded to 2. This will be generalized when tensor fields + // of other ranks are implemented in the system. + static constexpr std::size_t tensor_rank = 2; + using TensorFieldData_t = ResourcesManager::template UserTensorField_t<tensor_rank>::patch_data_type;src/amr/messengers/mhd_messenger.hpp (1)
115-119: Empty reflux method needs documentationThe newly added
refluxmethod is empty but marked asoverride, suggesting it's implementing an interface requirement. Consider adding a comment explaining why this implementation is empty.+ // MHD messenger does not require reflux operations as it maintains consistency + // through other mechanisms. This empty implementation satisfies the interface. void reflux(int const /*coarserLevelNumber*/, int const /*fineLevelNumber*/, double const /*syncTime*/) override { }src/amr/data/field/coarsening/field_coarsen_operator.hpp (2)
256-257: Fix typo in documentationThere's a typo in the documentation comment.
- * selected. Finnaly loop over the indexes in the box, and apply the coarsening defined + * selected. Finally loop over the indexes in the box, and apply the coarsening defined
273-294: Consider adding bounds checking for tensor component accessThe loop iterates over tensor components using index
c, but there's no explicit bounds checking.While the compile-time constant
Nshould prevent out-of-bounds access, consider adding a static_assert for clarity:for (std::uint16_t c = 0; c < N; ++c) { + static_assert(N > 0, "Tensor must have at least one component"); auto const& qty = destinationFields[c].physicalQuantity();src/amr/resources_manager/resources_manager.hpp (1)
467-472: Consider removing commented codeThe commented code appears to be an old implementation. If it's no longer needed, it should be removed entirely.
auto getResourcesNullPointer_(ResourcesInfo const& resourcesVariableInfo) const { - // using patch_data_type = ResourceType::patch_data_type; - // auto constexpr patch_data_ptr_fn = &patch_data_type::getPointer; - // using PointerType = std::invoke_result_t<decltype(patch_data_ptr_fn), - // patch_data_type>; - return nullptr; //.static_cast<PointerType>(nullptr); + return nullptr; }src/amr/resources_manager/resources_manager_utilities.hpp (1)
109-119: Consider reducing code duplication.The tensor field and field cases (lines 110-111 and 115-116) are identical, both creating variables with
name()andphysicalQuantity(). Consider combining these cases to reduce duplication.auto static make_shared_variable(ResourceView const& view) { - if constexpr (is_tensor_field_v<ResourceView>) - return std::make_shared<typename type::variable_type>(view.name(), - view.physicalQuantity()); - else if constexpr (is_particles_v<ResourceView>) + if constexpr (is_particles_v<ResourceView>) return std::make_shared<typename type::variable_type>(view.name()); - else if constexpr (is_field_v<ResourceView>) + else if constexpr (is_tensor_field_v<ResourceView> || is_field_v<ResourceView>) return std::make_shared<typename type::variable_type>(view.name(), view.physicalQuantity()); else throw std::runtime_error("bad condition"); }src/amr/data/tensorfield/tensor_field_variable.hpp (1)
33-42: Optimize duplicate computation of dataLivesOnPatchBorder.The
computeDataLivesOnPatchBorder_(qty)is called twice - once for the factory and once for the member variable. Consider computing it once and reusing the result.TensorFieldVariable(std::string const& name, tensor_t qty, bool fineBoundaryRepresentsVariable = false) - : SAMRAI::hier::Variable( - name, - std::make_shared<TensorFieldDataFactory<rank, GridLayoutT, Grid_t, PhysicalQuantity>>( - fineBoundaryRepresentsVariable, computeDataLivesOnPatchBorder_(qty), name, qty)) - , fineBoundaryRepresentsVariable_{fineBoundaryRepresentsVariable} - , dataLivesOnPatchBorder_{computeDataLivesOnPatchBorder_(qty)} + : SAMRAI::hier::Variable( + name, + [&]() { + bool dataLivesOnPatchBorder = computeDataLivesOnPatchBorder_(qty); + dataLivesOnPatchBorder_ = dataLivesOnPatchBorder; + return std::make_shared<TensorFieldDataFactory<rank, GridLayoutT, Grid_t, PhysicalQuantity>>( + fineBoundaryRepresentsVariable, dataLivesOnPatchBorder, name, qty); + }()) + , fineBoundaryRepresentsVariable_{fineBoundaryRepresentsVariable} { }src/amr/messengers/messenger.hpp (1)
13-13: Remove commented include.Commented code should be removed rather than left in the codebase.
-// #include "core/data/grid/gridlayout.hpp"tests/simulator/test_advance.py (3)
290-290: Move matplotlib import inside the exception handler.The matplotlib import is only used within the exception handler for debugging. Moving it inside would avoid the unused import warning when tests pass.
except AssertionError as e: - import matplotlib.pyplot as plt - from matplotlib.patches import Rectangle + import matplotlib.pyplot as plt + from matplotlib.patches import Rectangle
306-306: Rename ambiguous variable to improve readability.The variable name
lis ambiguous and could be confused with the number1.- (u - l) * d for u, l, d in zip(upper, lower, dl) + (u - low) * d for u, low, d in zip(upper, lower, dl)
659-661: Maintain consistent assertion style.The test uses
self.assertEqualthroughout, but this change introduces a plainassert. Consider maintaining consistency.- assert len(lvl_steps) == 2, ( - "this test is only configured for L0 -> L1 refinement comparisons" - ) + self.assertEqual( + len(lvl_steps), 2, + "this test is only configured for L0 -> L1 refinement comparisons" + )src/amr/data/field/coarsening/electric_field_coarsener.hpp (1)
154-154: Fix closing header guard to match the opening.The closing header guard comment should match the corrected opening guard.
-#endif +#endif // PHARE_ELECTRIC_FIELD_COARSENER_HPPsrc/amr/data/tensorfield/tensor_field_data_factory.hpp (1)
89-102: Document the use of dummy values in getBoxGeometry.The method uses dummy values for
dl(0.01) andorigin(0) while onlynbCellis meaningful. Consider adding a comment explaining why these dummy values are acceptable here.+ // Only nbCell is used by TensorFieldGeometry, dl and origin are placeholders std::array<double, dimension> dl; std::array<std::uint32_t, dimension> nbCell; core::Point<double, dimension> origin;src/amr/data/tensorfield/tensor_field_geometry.hpp (1)
109-110: Consider adding exception safety for dynamic_cast operations.The dynamic_cast operations could throw std::bad_cast if the types don't match. Consider adding try-catch blocks or validating types before casting.
Add exception handling:
-auto& destinationCast = dynamic_cast<TensorFieldGeometry const&>(destinationGeometry); -auto& sourceCast = dynamic_cast<TensorFieldGeometry const&>(sourceGeometry); +TensorFieldGeometry const* destinationCast = dynamic_cast<TensorFieldGeometry const*>(&destinationGeometry); +TensorFieldGeometry const* sourceCast = dynamic_cast<TensorFieldGeometry const*>(&sourceGeometry); + +if (!destinationCast || !sourceCast) +{ + throw std::runtime_error("TensorFieldGeometry::calculateOverlap: invalid geometry types"); +}src/amr/data/field/time_interpolate/field_linear_time_interpolate.hpp (1)
91-92: Remove empty comment line.The empty comment on line 91 serves no purpose and should be removed.
- // }src/amr/solvers/solver_ppc.hpp (1)
299-309: Consider performance optimization for field accumulation.The current implementation evaluates each component separately on the ghost box. This could be optimized by combining the operations.
Consider using a single loop over the ghost box indices:
-layout.evalOnGhostBox(fluxSumE_(core::Component::X), [&](auto const&... args) mutable { - fluxSumE_(core::Component::X)(args...) += Eavg(core::Component::X)(args...) * coef; -}); - -layout.evalOnGhostBox(fluxSumE_(core::Component::Y), [&](auto const&... args) mutable { - fluxSumE_(core::Component::Y)(args...) += Eavg(core::Component::Y)(args...) * coef; -}); - -layout.evalOnGhostBox(fluxSumE_(core::Component::Z), [&](auto const&... args) mutable { - fluxSumE_(core::Component::Z)(args...) += Eavg(core::Component::Z)(args...) * coef; -}); +auto ghostBox = layout.getGhostBox(); +layout.evalOnBox(ghostBox, [&](auto const&... args) mutable { + fluxSumE_(core::Component::X)(args...) += Eavg(core::Component::X)(args...) * coef; + fluxSumE_(core::Component::Y)(args...) += Eavg(core::Component::Y)(args...) * coef; + fluxSumE_(core::Component::Z)(args...) += Eavg(core::Component::Z)(args...) * coef; +});src/amr/data/tensorfield/tensor_field_data.hpp (1)
32-35: Remove unused TensorFieldDataInternals class template.The
TensorFieldDataInternalsclass template appears to be an unused placeholder. If it's not needed for future specializations, it should be removed to reduce code complexity.Consider removing this unused template class if it's not part of a planned extension.
src/amr/data/field/refine/magnetic_refine_patch_strategy.hpp (2)
73-74: Consider more descriptive variable names for structured binding.While
bx, by, bzare clear in this context, using the actual field names would be more maintainable.-auto& [bx, by, bz] = fields; +auto& [magnetic_x, magnetic_y, magnetic_z] = fields;Or document the binding:
+// Extract magnetic field components: Bx, By, Bz auto& [bx, by, bz] = fields;
42-42: Improve assertion message for clarity.The assertion message could be more specific about which ID needs to be registered.
-assert(b_id_ >= 0 && "MagneticRefinePatchStrategy: IDs must be registered before use"); +assert(b_id_ >= 0 && "MagneticRefinePatchStrategy: magnetic field tensor ID must be registered before use");src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp (1)
174-210: Improve error messages with specific variable namesThe error messages when IDs are missing could be more specific about which variable failed to retrieve.
- if (!b_id) - { - throw std::runtime_error( - "HybridHybridMessengerStrategy: missing magnetic field variable IDs"); - } + if (!b_id) + { + throw std::runtime_error( + "HybridHybridMessengerStrategy: failed to get ID for magnetic field variable: " + + hybridInfo->modelMagnetic); + } // ... similar changes for other ID checks ... - if (!e_id) - { - throw std::runtime_error( - "HybridHybridMessengerStrategy: missing electric field variable IDs"); - } + if (!e_id) + { + throw std::runtime_error( + "HybridHybridMessengerStrategy: failed to get ID for electric field variable: " + + hybridInfo->modelElectric); + } - if (!e_reflux_id or !e_fluxsum_id) - { - throw std::runtime_error( - "HybridHybridMessengerStrategy: missing electric refluxing field variable IDs"); - } + if (!e_reflux_id or !e_fluxsum_id) + { + throw std::runtime_error( + "HybridHybridMessengerStrategy: failed to get ID for refluxing variables: " + + (!e_reflux_id ? hybridInfo->refluxElectric : "") + + (!e_fluxsum_id ? " " + hybridInfo->fluxSumElectric : "")); + }
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/amr/data/field/refine/magnetic_refine_patch_strategy.hpp
Dismissed
Show dismissed
Hide dismissed
| auto& by = FieldDataT::getField(fine, by_id_); | ||
| auto& bz = FieldDataT::getField(fine, bz_id_); | ||
| auto& fields = TensorFieldDataT::getFields(fine, b_id_); | ||
| auto& [bx, by, bz] = fields; |
Check notice
Code scanning / CodeQL
Unused local variable Note
| auto& J = hybridModel.state.J; | ||
| auto& Vi = hybridModel.state.ions.velocity(); | ||
| auto& Ni = hybridModel.state.ions.chargeDensity(); | ||
| auto& E = hybridModel.state.electromag.E; |
Check notice
Code scanning / CodeQL
Unused local variable Note
|
|
||
|
|
||
| TYPED_TEST(SimulatorTest, knowsWhichSolverisOnAGivenLevel) | ||
| TYPED_TEST(SimulatorTest, knowsWhichSolverIsOnAGivenLevel) |
Check notice
Code scanning / CodeQL
Unused static variable Note test
| // auto& mhdModel = *sim.getMHDModel(); | ||
| // |
Check notice
Code scanning / CodeQL
Commented-out code Note test
| // if (isInMHDdRange(iLevel)) | ||
| // { | ||
| // auto Bid = mhdModel.resourcesManager->getIDs(mhdModel.state.B); | ||
| // auto Vid = mhdModel.resourcesManager->getIDs(mhdModel.state.V); | ||
| // | ||
| // std::array<std::vector<int> const*, 2> allIDs{{&Bid, &Vid}}; | ||
| // | ||
| // for (auto& idVec : allIDs) | ||
| // { | ||
| // for (auto& id : *idVec) | ||
| // { | ||
| // auto level = hierarchy.getPatchLevel(iLevel); | ||
| // auto patch = level->begin(); | ||
| // EXPECT_TRUE(patch->checkAllocated(id)); | ||
| // } | ||
| // } | ||
| // } |
Check notice
Code scanning / CodeQL
Commented-out code Note test
|
|
||
|
|
||
| TYPED_TEST(SimulatorTest, returnsCorrecMessengerForEachLevel) | ||
| TYPED_TEST(SimulatorTest, returnsCorrectMessengerForEachLevel) |
Check notice
Code scanning / CodeQL
Unused static variable Note test
nicolasaunai
changed the title
UCaromel
force-pushed
the
fix-master
branch
3 times, most recently
from
132c76b to
5231eec
Compare
- fine_boundary_represents_var for copy **before** refinement - PatchData NaN initialized on construction - fix tests failing as result of above - comment a field refinement test (useless, wrong refinement op for E,B) - debug plots for advance field overlap test - copy done before refinement (boolean false in variable) - overwrite_interior false also for refinement is default for FieldFillPattern - J manually init to zero in model init, fine init and regrid init (Jx unused in ampere but used in Ohm with its now NaN values) - Grid/NdarrayVector take default value overrides (for test) - UsableTensorField is default constructed with zero init. - TensorFieldData - with refluxing
| template<typename Dst> | ||
| void linear_time_interpolate(Dst& fieldDest, auto& fieldSrcOld, auto& fieldSrcNew, auto&&... args) | ||
| { | ||
| auto const& [localDestBox, localSrcBox, alpha] = std::forward_as_tuple(args...); |
Check notice
Code scanning / CodeQL
Unused local variable Note
| // magPatchGhostsRefineSchedules[levelNumber]->fillData(initDataTime); | ||
| // elecPatchGhostsRefineSchedules[levelNumber]->fillData(initDataTime); |
Check notice
Code scanning / CodeQL
Commented-out code Note
|
PR comments addressed and pushed as #1075 |
3 participants
Uh oh!
There was an error while loading. Please reload this page.