search

geodynamics / aspect

A parallel, extensible finite element code to simulate convection in both 2D and 3D models.
https://aspect.geodynamics.org/
Other
228 stars 237 forks source link

Rename particle world to particle manager #6058

Closed gassmoeller closed 1 month ago

gassmoeller commented 1 month ago

This is a suggestion resulting from the discussion in the user meeting yesterday about a better name for the particle world class. See my reasoning for the renaming here. I think the most convincing suggestion was to try the name Manager, because it most directly explains the purpose of the class, and it is nicely analogous to all the existing manager classes we have already. Part of me is sad to see World go, because the name had something poetic about it (all the particles living in their own world :smile:). But I think the new term is better, and the implementation was simpler than expected. I even found a way to make moving the files and renaming the class backward compatible by keeping the main header file around and by declaring an alias for the World class name. This PR is mostly a lot of search and replace, but in essence it does:

  1. Rename all instances of world to manager, incl. the Particle::World class name, the simulator access functions, and the new cmake variable and input/output parameters.
  2. Move the files world.[h,cc] to manager.[h,cc]. Also rename all existing test files.
  3. Add a new world.h header file that enables backward compatibility for existing user plugins.
  4. Add an update script that should update calls to simulator_access.get_particle_world() to the new simulator_access.get_particle_manager(0). I assumed we would mostly update files from before #5920 which introduced the new function parameter.
  5. I moved the input parameter Number of particle managers from the global subsection into the Particles subsection. I think this is more appropriate and aligns closer with how things are done for compositional fields (Compositional fields/Number of compositional fields).

@MFraters, @tjhei, @bangerth: You were most closely involved in the discussion, can you take a look? It is a lot of changes, but almost all of it is simple search/replace.

tjhei commented 1 month ago

I am not sure I expressed my opinion on the naming well yesterday (for what it is worth, I don't feel strongly about it). For me: "particle system" > "particle world" > "particle manager".

The word "manager" doesn't make it obvious to me why you would want more than one of them.

gassmoeller commented 1 month ago

I see your point from the user (input/output) perspective. However, I think System is not a good name for the class, because as Wolfgang mentioned the purpose of the class is the active process of managing the particles (a lot of the class documentation already spoke about managing the particles before my PR).

Do you think it would be confusing if we call the input/output options particle "system" ("Number of particle systems", "Particle system 2", ...), and keep particle "manager" for everything inside the source code ("this->get_particle_manager(x)")? This would make some sense to me as in: If the user requests multiple particle systems in their model, we need multiple particle manager objects to manage those systems.

tjhei commented 1 month ago

Sure, fine with me.

gassmoeller commented 1 month ago

Ok, I implemented that and updated the test input and output in the last commit. Is this a version we can all agree on @bangerth @tjhei @MFraters (and anyone else who reads this)? Then I can rebase and also rename the tests (from multiple_managers to multiple_systems).

bangerth commented 1 month ago

OK to merge from my side, but you've got a bunch of merge conflicts.