Skip to content
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

Collect common solver functionality in a base class #228

Merged
merged 38 commits into from
May 8, 2024

Conversation

oparry-ukaea
Copy link
Contributor

@oparry-ukaea oparry-ukaea commented Apr 8, 2024

Description

Creates a base class for solver equation systems from which H3LAPD (DriftReducedSystem) and SimpleSOL (SOLSystem) can inherit. Base class is templated on a nektar equation system (for time-evolving systems, it must derive from UnsteadySystem). Time evolving systems are also templated on a particle system class, which must derive from a new particle system base class.

Type of change

  • Refactoring
  • New feature (non-breaking change which adds functionality)
  • Requires documentation updates

Testing

None added

Checklist:

  • New and existing unit tests pass locally with my changes
  • Any new dependencies are automatically built for users via cmake
  • I have used understandable variable names
  • I have run clang-format against my *.hpp and *.cpp changes
  • I have run cmake-format against my changes to CMakeLists.txt
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

@oparry-ukaea oparry-ukaea added this to the neso-hw-impurity-transport milestone Apr 11, 2024
@oparry-ukaea oparry-ukaea self-assigned this Apr 12, 2024
@oparry-ukaea oparry-ukaea removed this from the neso-hw-impurity-transport milestone Apr 12, 2024
@jwscook
Copy link
Member

jwscook commented May 1, 2024

Once ExCALIBUR-NEPTUNE/NESO-hw-impurity-transport#5 is merged then this can follow quite quickly

@jwscook
Copy link
Member

jwscook commented May 1, 2024

Should run this past Stefan and Sanket before merging because this assumes that there is a single ParticleGroup per ParticleSystem, which may not fit their usecase.

@jwscook
Copy link
Member

jwscook commented May 1, 2024

Need to create some instructions in the README after NESO 0.1.0.

@oparry-ukaea
Copy link
Contributor Author

Having had another look, I don't think it'll be a problem to adapt the particle system base class to work with multiple ParticleGroups in a separate PR if needs be. Marking this as ready-to-merge.

@oparry-ukaea
Copy link
Contributor Author

Instructions for using the base classes in a new solver need to be added (#239), but that may depend on other changes (rearranging submodules etc.) to some extent - I propose we do that in a separate PR.

Copy link
Contributor

@will-saunders-ukaea will-saunders-ukaea left a comment

Choose a reason for hiding this comment

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

Built and tests passed.

@oparry-ukaea oparry-ukaea removed the Awaiting Review PR waiting to be reviewed. label May 8, 2024
@oparry-ukaea oparry-ukaea merged commit c03be8d into main May 8, 2024
@oparry-ukaea oparry-ukaea deleted the feature/basesolver branch May 8, 2024 12:50
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.

3 participants