Skip to content

drop ppc model view #1178

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
PhilipDeegan wants to merge 3 commits into
base: master
Choose a base branch
from
Open

drop ppc model view #1178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b5faac9
drop ppc model view
PhilipDeegan Mar 23, 2026
5ec2a93
drop LayoutHolder
PhilipDeegan Apr 3, 2026
9268d21
level transformer
PhilipDeegan Apr 3, 2026
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
42 changes: 15 additions & 27 deletions src/amr/level_initializer/hybrid_level_initializer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@
#define PHARE_HYBRID_LEVEL_INITIALIZER_HPP

#include "core/errors.hpp"
#include "core/numerics/ohm/ohm.hpp"
#include "core/utilities/mpi_utils.hpp"
#include "core/numerics/ampere/ampere.hpp"
#include "core/numerics/moments/moments.hpp"
#include "core/data/grid/gridlayout_utils.hpp"
#include "core/numerics/interpolator/interpolator.hpp"

#include "amr/messengers/messenger.hpp"
#include "amr/messengers/hybrid_messenger.hpp"
#include "amr/resources_manager/amr_utils.hpp"
#include "amr/solvers/solver_field_evolvers.hpp"
#include "amr/physical_models/physical_model.hpp"
#include "amr/level_initializer/level_initializer.hpp"

Expand All @@ -37,16 +35,18 @@ namespace solver
static constexpr auto dimension = GridLayoutT::dimension;
static constexpr auto interp_order = GridLayoutT::interp_order;

PHARE::core::Ohm<GridLayoutT> ohm_;
PHARE::core::Ampere<GridLayoutT> ampere_;
using Ampere_t = AmpereLevelTransformer<HybridModel>;
using Ohm_t = OhmLevelTransformer<HybridModel>;
core::OhmInfo ohm_info;

inline bool isRootLevel(int const levelNumber) const { return levelNumber == 0; }

public:
explicit HybridLevelInitializer(PHARE::initializer::PHAREDict const& dict)
: ohm_{dict["algo"]["ohm"]}
: ohm_info{core::OhmInfo::FROM(dict["algo"]["ohm"])}
{
}

virtual void initialize(std::shared_ptr<hierarchy_t> const& hierarchy, int levelNumber,
std::shared_ptr<level_t> const& oldLevel, IPhysicalModelT& model,
amr::IMessenger<IPhysicalModelT>& messenger, double initDataTime,
Expand Down Expand Up @@ -150,34 +150,22 @@ namespace solver
if (!isRegriddingL0)
if (isRootLevel(levelNumber))
{
TimeSetter setTime{level, hybridModel, 0.};

auto& B = hybridModel.state.electromag.B;
auto& E = hybridModel.state.electromag.E;
auto& J = hybridModel.state.J;

for (auto& patch : rm.enumerate(level, B, J))
{
auto layout = PHARE::amr::layoutFromPatch<GridLayoutT>(*patch);
auto __ = core::SetLayout(&layout, ampere_);
ampere_(B, J);

rm.setTime(J, *patch, 0.);
}
Ampere_t{level, hybridModel}(B, J);
setTime(J);
hybMessenger.fillCurrentGhosts(J, level, 0.);

auto& electrons = hybridModel.state.electrons;
auto& E = hybridModel.state.electromag.E;

for (auto& patch : rm.enumerate(level, B, E, J, electrons))
{
auto layout = PHARE::amr::layoutFromPatch<GridLayoutT>(*patch);
electrons.update(layout);
auto& Ve = electrons.velocity();
auto& Ne = electrons.density();
auto& Pe = electrons.pressure();
auto __ = core::SetLayout(&layout, ohm_);
ohm_(Ne, Ve, Pe, B, J, E);
rm.setTime(E, *patch, 0.);
}
for (auto& patch : rm.enumerate(level, electrons))
electrons.update(amr::layoutFromPatch<GridLayoutT>(*patch));

Ohm_t{ohm_info, level, hybridModel}(B, J, E, electrons);
setTime(E);
hybMessenger.fillElectricGhosts(E, level, 0.);
Comment on lines +153 to 169
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Propagate initDataTime instead of hard-coding 0..

Line 155 stamps the TimeSetter with 0., and Lines 163 and 171 use the same literal for the ghost fills. That regresses any root-level initialization performed at nonzero initDataTime by publishing J and E at the wrong time.

🩹 Minimal fix 
-                    TimeSetter setTime{level, hybridModel, 0.};
+                    TimeSetter setTime{level, hybridModel, initDataTime};
...
-                    hybMessenger.fillCurrentGhosts(J, level, 0.);
+                    hybMessenger.fillCurrentGhosts(J, level, initDataTime);
...
-                    hybMessenger.fillElectricGhosts(E, level, 0.);
+                    hybMessenger.fillElectricGhosts(E, level, initDataTime);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/amr/level_initializer/hybrid_level_initializer.hpp` around lines 155 -
171, The TimeSetter and ghost-fill calls are hard-coded with 0.; instead
propagate the actual initialization time by using initDataTime: construct
TimeSetter setTime{level, hybridModel, initDataTime} and pass initDataTime to
hybMessenger.fillCurrentGhosts(J, level, initDataTime) and
hybMessenger.fillElectricGhosts(E, level, initDataTime); keep the ampere_ and
ohm_ calls and the setTime(J)/setTime(E) calls but ensure they use the
TimeSetter that was initialized with initDataTime.

Copy link
Copy Markdown
Member Author

@PhilipDeegan PhilipDeegan Apr 3, 2026

Choose a reason for hiding this comment

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

was it already like that?

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

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

@PhilipDeegan — yes, it was already like that. The removed lines in the diff already used rm.setTime(J, *patch, 0.) and rm.setTime(E, *patch, 0.), and the ghost-fill calls were unchanged context lines already passing 0.. This PR preserved the pre-existing behavior; it didn't introduce the hard-coding. My concern is about a pre-existing pattern rather than a regression from this PR. Apologies for the noise.

🧠 Learnings used 
Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 784
File: tests/simulator/test_restarts.py:333-339
Timestamp: 2024-10-09T08:32:15.667Z
Learning: PhilipDeegan has indicated a preference for minimal and efficient code, even after making changes to get a function working. There may be opportunities to remove or optimize parts of the code such as sleep/wait/advances.

Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 784
File: tests/simulator/test_restarts.py:333-339
Timestamp: 2024-07-26T22:04:34.160Z
Learning: PhilipDeegan has indicated a preference for minimal and efficient code, even after making changes to get a function working. There may be opportunities to remove or optimize parts of the code such as sleep/wait/advances.

Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 1068
File: src/amr/data/field/coarsening/electric_field_coarsener.hpp:1-2
Timestamp: 2025-09-17T13:35:11.533Z
Learning: PhilipDeegan prefers header guard names that include the full directory path structure, following the pattern PHARE_[PATH_WITH_UNDERSCORES]_HPP. For example, a file at src/amr/data/field/coarsening/electric_field_coarsener.hpp should use PHARE_AMR_DATA_FIELD_COARSENING_ELECTRIC_FIELD_COARSENER_HPP as the header guard.

Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 888
File: src/hdf5/detail/h5/h5_file.hpp:78-79
Timestamp: 2024-09-05T17:02:58.784Z
Learning: Avoid commenting on draft PRs for the user PhilipDeegan.

Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 888
File: src/hdf5/detail/h5/h5_file.hpp:78-79
Timestamp: 2024-10-09T08:32:15.667Z
Learning: Avoid commenting on draft PRs for the user PhilipDeegan.

Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 888
File: pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py:566-568
Timestamp: 2024-09-09T13:57:02.285Z
Learning: Avoid commenting on draft PRs for PhilipDeegan unless explicitly requested.

Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 1158
File: src/amr/data/particles/refine/split_3d.hpp:66-87
Timestamp: 2026-02-25T13:09:17.840Z
Learning: In the PHARE codebase, for runtime particle-based operations, avoid lambdas with captures due to performance issues. While constexpr lambdas are equivalent to functors, runtime particle operations benefit from using explicit non-captured functor classes instead of lambdas with capture. Apply this guideline to C++ header files (e.g., src/.../*.hpp) involved in particle processing to ensure predictable inlining and performance.

Learnt from: PhilipDeegan
Repo: PHAREHUB/PHARE PR: 1177
File: src/hdf5/detail/h5/h5_file.hpp:116-119
Timestamp: 2026-03-23T13:52:54.931Z
Learning: In PHAREHUB/PHARE, MPI initialization is the responsibility of the calling application—PHARE code should not call MPI_Init or MPI_Init_thread. For HDF5 subfiling paths that require MPI_THREAD_MULTIPLE, PHARE should rely on the MPI implementation’s configured thread support (e.g., OMPI_MPI_THREAD_LEVEL via environment variable, or an equivalent mechanism) rather than attempting to initialize MPI with MPI_Init_thread inside PHARE.

}

Expand Down
15 changes: 1 addition & 14 deletions src/amr/multiphysics_integrator.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,6 @@ namespace solver
}
else
load_balancer_manager_->estimate(*level, model);

if (static_cast<std::size_t>(levelNumber) == model_views_.size())
model_views_.push_back(solver.make_view(*level, model));
else
model_views_[levelNumber] = solver.make_view(*level, model);
}


Expand All @@ -391,9 +386,6 @@ namespace solver
{
messenger.registerLevel(hierarchy, ilvl);

model_views_.push_back(getSolver_(ilvl).make_view(
AMR_Types::getLevel(*hierarchy, ilvl), getModel_(ilvl)));

auto level = hierarchy->getPatchLevel(ilvl);
for (auto& patch : *level)
{
Expand Down Expand Up @@ -533,8 +525,7 @@ namespace solver
solver.prepareStep(model, *level, currentTime);
fromCoarser.prepareStep(model, *level, currentTime);

solver.advanceLevel(*hierarchy, iLevel, getModelView_(iLevel), fromCoarser, currentTime,
newTime);
solver.advanceLevel(*hierarchy, iLevel, model, fromCoarser, currentTime, newTime);

if (lastStep)
{
Expand Down Expand Up @@ -648,8 +639,6 @@ namespace solver
std::vector<std::unique_ptr<ISolver<AMR_Types>>> solvers_;
std::vector<std::shared_ptr<IPhysicalModel<AMR_Types>>> models_;

std::vector<std::shared_ptr<ISolverModelView>> model_views_;

std::vector<std::shared_ptr<PHARE::amr::Tagger>> taggers_;
std::map<std::string, std::unique_ptr<IMessengerT>> messengers_;
std::map<std::string, std::unique_ptr<LevelInitializerT>> levelInitializers_;
Expand Down Expand Up @@ -877,8 +866,6 @@ namespace solver
}


auto& getModelView_(int iLevel) { return *model_views_[iLevel]; }


IPhysicalModel<AMR_Types>& getModel_(int iLevel)
{
Expand Down
15 changes: 9 additions & 6 deletions src/amr/physical_models/hybrid_model.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
#ifndef PHARE_HYBRID_MODEL_HPP
#define PHARE_HYBRID_MODEL_HPP

#include <string>

#include "initializer/data_provider.hpp"
#include "core/def.hpp"
#include "core/models/hybrid_state.hpp"
#include "amr/physical_models/physical_model.hpp"
#include "core/data/ions/particle_initializers/particle_initializer_factory.hpp"
#include "amr/resources_manager/resources_manager.hpp"

#include "initializer/data_provider.hpp"

#include "amr/physical_models/physical_model.hpp"
#include "amr/messengers/hybrid_messenger_info.hpp"
#include "core/data/vecfield/vecfield.hpp"
#include "core/def.hpp"
#include "amr/resources_manager/resources_manager.hpp"


#include <string>

namespace PHARE::solver
{
Expand Down
28 changes: 28 additions & 0 deletions src/amr/physical_models/physical_model.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,32 @@ namespace solver

} // namespace PHARE



namespace PHARE::solver
{


template<typename level_t, typename Model>
struct TimeSetter
{
void operator()(auto&... quantities)
{
auto& rm = *model.resourcesManager;
for (auto& patch : rm.enumerate(level, quantities...))
(model.resourcesManager->setTime(quantities, *patch, newTime), ...);
}

level_t& level;
Model& model;
double newTime;
};

template<typename level_t, typename Model>
TimeSetter(level_t&, Model&, double) -> TimeSetter<level_t, Model>;


} // namespace PHARE::solver


#endif
29 changes: 28 additions & 1 deletion src/amr/resources_manager/resources_manager.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ namespace amr
template<typename Level_t, typename... Args>
struct LevelLooper
{
using value_type = std::shared_ptr<SAMRAI::hier::Patch>;

LevelLooper(ResourcesManager& rm, Level_t& lvl, Args&... arrgs)
: rm{rm}
, level{lvl}
Expand All @@ -351,6 +353,28 @@ namespace amr
}


struct Patch
{
Patch(LevelLooper* looper, std::shared_ptr<SAMRAI::hier::Patch> const& patch)
: looper{looper}
, patch{patch}
{
looper->set(*patch);
}

Patch(Patch const&) = delete;
Patch(Patch&&) = delete;
Patch& operator=(Patch const&) = delete;
Patch& operator=(Patch&&) = delete;

SAMRAI::hier::Patch const& operator*() { return *patch; }

~Patch() { looper->unset(); }

LevelLooper* looper;
std::shared_ptr<SAMRAI::hier::Patch> const patch;
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

struct Iterator
{
void operator++() { ++raw; }
Expand Down Expand Up @@ -380,6 +404,8 @@ namespace amr

auto begin() { return Iterator{this, level.begin()}; }
auto end() { return Iterator{this, level.end()}; };
auto size() const { return static_cast<std::size_t>(level.getLocalNumberOfPatches()); }
auto operator[](std::size_t const idx) { return Patch{this, level.getPatch(idx)}; }

ResourcesManager& rm;
Level_t& level;
Expand Down Expand Up @@ -411,7 +437,8 @@ namespace amr

if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
{
// unpack the tuple subResources and apply for each element registerResources()
// unpack the tuple subResources and apply for each element
// registerResources()
std::apply(
[this, &IDs](auto&... subResource) {
(this->getIDs_(subResource, IDs), ...);
Expand Down
46 changes: 13 additions & 33 deletions src/amr/solvers/solver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,6 @@

#include <string>

namespace PHARE::solver
{


class ISolverModelView
{
public:
using This = ISolverModelView;

virtual ~ISolverModelView() = default;
};


} // namespace PHARE::solver



Expand All @@ -45,9 +31,10 @@ namespace solver
class ISolver
{
public:
using patch_t = typename AMR_Types::patch_t;
using level_t = typename AMR_Types::level_t;
using hierarchy_t = typename AMR_Types::hierarchy_t;
using patch_t = AMR_Types::patch_t;
using level_t = AMR_Types::level_t;
using hierarchy_t = AMR_Types::hierarchy_t;
using IPhysicalModel_t = IPhysicalModel<AMR_Types>;

/**
* @brief return the name of the ISolver
Expand All @@ -69,7 +56,7 @@ namespace solver
* IPhysicalModel
* @param model
*/
virtual void registerResources(IPhysicalModel<AMR_Types>& model) = 0;
virtual void registerResources(IPhysicalModel_t& model) = 0;



Expand All @@ -87,40 +74,36 @@ namespace solver
* called before the advanceLevel() method.
*
*/
virtual void prepareStep(IPhysicalModel<AMR_Types>& model, SAMRAI::hier::PatchLevel& level,
double const currentTime)
virtual void prepareStep(IPhysicalModel_t& model, level_t& level, double const currentTime)
= 0;

/**
* @brief accumulateFluxSum accumulates the flux sum(s) on the given PatchLevel for
* refluxing later.
*/
virtual void accumulateFluxSum(IPhysicalModel<AMR_Types>& model,
SAMRAI::hier::PatchLevel& level, double const coef)
virtual void accumulateFluxSum(IPhysicalModel_t& model, level_t& level, double const coef)
= 0;


/**
* @brief resetFluxSum resets the flux sum(s) on the given PatchLevel to zero.
*/
virtual void resetFluxSum(IPhysicalModel<AMR_Types>& model, SAMRAI::hier::PatchLevel& level)
= 0;
virtual void resetFluxSum(IPhysicalModel_t& model, level_t& level) = 0;


/**
* @brief implements the reflux operations needed for a given solver.
*/
virtual void reflux(IPhysicalModel<AMR_Types>& model, SAMRAI::hier::PatchLevel& level,
amr::IMessenger<IPhysicalModel<AMR_Types>>& messenger,
double const time)
virtual void reflux(IPhysicalModel_t& model, level_t& level,
amr::IMessenger<IPhysicalModel_t>& messenger, double const time)
= 0;

/**
* @brief advanceLevel advances the given level from t to t+dt
*/
virtual void advanceLevel(hierarchy_t const& hierarchy, int const levelNumber,
ISolverModelView& view,
amr::IMessenger<IPhysicalModel<AMR_Types>>& fromCoarser,
IPhysicalModel_t& view,
amr::IMessenger<IPhysicalModel_t>& fromCoarser,
double const currentTime, double const newTime)
= 0;

Expand All @@ -131,7 +114,7 @@ namespace solver
* @brief allocate is used to allocate ISolver variables previously registered to the
* ResourcesManager of the given model, onto the given Patch, at the given time.
*/
virtual void allocate(IPhysicalModel<AMR_Types>& model, patch_t& patch,
virtual void allocate(IPhysicalModel_t& model, patch_t& patch,
double const allocateTime) const
= 0;

Expand All @@ -143,9 +126,6 @@ namespace solver
virtual ~ISolver() = default;


virtual std::shared_ptr<ISolverModelView> make_view(level_t&, IPhysicalModel<AMR_Types>&)
= 0;

protected:
explicit ISolver(std::string name)
: solverName{std::move(name)}
Expand Down
Loading
Loading