puncproject / PUNCpp

Particles-in-UNstructured-Cells, C++ version
https://punc.readthedocs.io
GNU General Public License v3.0
12 stars 6 forks source link

Clean interaction.cpp #18

Open sigvaldm opened 6 years ago

diakodarian commented 6 years ago

Did some cleaning. Still some work remains.

sigvaldm commented 6 years ago

This is really the symptom of several issues, since interaction.cpp cannot be cleaned before some of the underlying structures are. It requires taking care of the following issues:

https://github.com/puncproject/PUNCpp/issues/14 https://github.com/puncproject/PUNCpp/issues/32 https://github.com/puncproject/PUNCpp/issues/33

sigvaldm commented 6 years ago

This one is suitable to fix at the same time as fixing run(): https://github.com/puncproject/PUNCpp/issues/34

sigvaldm commented 6 years ago

n_state input not implemented, so I'll remove until it's implemented.

sigvaldm commented 6 years ago

I have cleaned up the argument list but interaction.cpp is getting seriously messy and hard to maintain. Never add new features to this file without cleaning up properly. Some notes for later cleaning:

Way too much goes on in the run function. It probably needs to be divided into convenient subroutines, for instance:

species = setup_species(options)

Auxiliary functions like setup_species should probably get their own file, but they are not part of PUNC, only interaction. We may also need some auxiliary functions for dealining with input. I'm working with this.

We probably also need to rename about half the variables. Examples:

Some variables which are floating around actually belongs to some "concept" and should be encapsulated by that concept. For instance ni_ema and ne_ema, and the dirichlet boundary condition. The ema filter could be a class storing this in private members. It is of no use outside of this "concept". Inside interaction we are mostly concerned with control flow, and strive to encapsulate irrelevant things other places.

EDIT: I must have been sleepy when looking at this. ne_ema and ni_ema are good as they are. At the moment I thought they were internal states but they are in fact output, and thus are better off as they are.

Objects are somewhat adhoc with the impose_ variables. I will work on this soon.

diakodarian commented 6 years ago

I agree some of the names are undescriptive. When it comes to function spaces, I have consistently used the following naming convention throughout the code:

V as CG1 function space W as CG1 vector function space Q as DG0 function space P as DG0 vector space.

I suggest we keep this naming convention internally inside the code. In the interface (interaction.cpp), we can use more descriptive names.

sigvaldm commented 6 years ago

I agree. The naming inside small functions is less important (the smaller the scope, the less descriptive the name needs to be. Consider for instance i for loop counters). Besides, it's not worth going through every little function.

But rather than giving a name for "CG1 function space" and so on, I suggest giving a name for phi, rho and E's function spaces, regardless of whether these are DG0, CG1, etc. What do you think about these: rho_space, phi_space, E_space. If we decide to change phi to be CG2, we change the value only of phi_space. As far as interaction.cpp is concerned, these are the only spaces need. If some E-field solvers need an intermediate space for doing some projection, then this can be local and interaction.cpp need not know about it, right?

sigvaldm commented 6 years ago

Got rid of the terrible argument list in interaction.cpp.

diakodarian commented 6 years ago

Nice!

sigvaldm commented 6 years ago

In cleaning up the input system, I went through the variables in the input file and gave many of them new names.

sigvaldm commented 6 years ago

Created read_species() as disscuessed above. One could also implement read_objects() and similar for other (huge) sections in the ini-file.