Skip to content

Changing Template variable in PicManager #287

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.

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
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JoMee
Copy link
Contributor

@JoMee JoMee commented May 1, 2024

I changed the template variable of the field solver class to be more generic. One needs this to for example run multiple solvers in one step. I don't see a reason why this is not generic, I also updated the alpine manager and the simulations ran without problem.

…his could be useful if one wants to run multiple solvers in one step
@manuel5975p
Copy link
Contributor

I could be wrong, but if the pointer type is going to be a template parameter why would we not remove the polymorphism altogether? Going from
std::shared_ptr<solver_base> to simply a solver_type?

@srikrrish srikrrish requested a review from matt-frey May 5, 2024 17:51
@JoMee
Copy link
Contributor Author

JoMee commented Mar 18, 2025

That depends if we want to support switching out the solver at runtime in the future (as we currently do with the FieldContainer or ParticleContainer). I don't think the overhead of this attribute is going to be too large.
Another point is if we want to return a pointer to the field solver at some point then it would be correct to have it as a shared pointer. (Since if the PICManager is destructed, the field solver would also get destructed)

In summary, it is probably the more future-proof way and doesn't add significant overhead, this is why I would keep it in a shared_ptr.

@aaadelmann aaadelmann requested review from aaadelmann and removed request for manuel5975p and matt-frey March 18, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants