Restructuring the files on the main repo

[rsanfer]

Another important topic from the meeting was the need to restructure the files on the main repository, to reduce their sizes and improve both readability and compilation times.

Of course, any file movement will affect the developers with ongoing branches (see #582) so one suggestion is to restructure one file at a time, maybe once every month or so, to reduce the burden to everybody.

I have a number of files in my radar that are priorities:

• driver_structure.cpp (I already have a branch ongoing on this, as some of you know)
• iteration_structure.cpp
• config_structure.cpp
• geometry_structure.cpp
  because of their length and the mixture of class structures. There is also a need to improve the includes throughout the code.

I would like to collect your thoughts.

[rsanfer]

On an added note to this topic, and to avoid some future hassle, I have collected a list of active branches (updated within the last 3 months) that are currently falling far behind the last release:

• feature_boom
• feature_caa
• feature_turbo
• feature_turbo2phase
• feature_adjoint_lut
• feature_transp_part
• feature_turbo_aeroelastic
• feature_gradientNorm

I would strongly encourage to regularly update with develop to ensure the future merging process is as smooth as possible, particularly when we start moving files around.

[pcarruscag]

In terms of file organisation I can think of two strategies, one would be grouping them according to parent class, numerics, solver, etc. this would maybe make navigation easier for people who work on the code daily but it does little more than what the naming convention already does. The other would be to group families of classes and that, I think, would help people who are less familiar with the code, so for example numerics, solvers, variables, etc. used for fluid simulations would be separate from the ones used for other physics.

The main re-compilation trigger is probably the config, every file includes it and not all need it. Not much can be done here because most classes do use the config in some way but we could try to minimise how much the dependency spreads by:

• Reviewing what cpp files actually need to include config_structure.hpp
• Including from the cpp and using a forward declaration on the corresponding hpp

Other sensitive "triggers" are the header files of classes with many children (solver, numerics, and variable) splitting the header files for these (maybe by family for small classes to avoid an explosion of hpp files) would allow working on a child class without re-compiling all others so often. The clients of CSolver would only include the header for the abstract class so those too would be shielded from changes in children classes.

[economon]

Nice post @pcarruscag : I think we can gain a lot by first breaking up the files, especially the headers, and then being much better about include statements. In particular, we can look at forward declarations, using more guards, and just minimizing the number of includes (I expect some are redundant).

As far as breaking up the files, I think geometry is a good place to start. I can take a shot at this first soon.

More ideas out there? Anyone have some bandwidth to experiment a little with the headers?

[economon]

Bumping this thread...

Files are now starting to be divided in several PRs, but we should set a clear policy for this. Seems that the consensus is to carry one class per file, move the inlines to the headers, and create subfolders where possible (i.e., have a more flat hierarchy in the src directory based on logical groups such as numerics, geometry, solvers, etc). We do not have a clear naming convention yet, but if it is one class per file, then an option is simply the class name.

Am I capturing the current consensus correctly? Anything I am missing? Naming preferences?

[talbring]

Yep these are the things I also got so far. If anyone has better idea, please let us know! 👍

[clarkpede]

One comment I have-- @pcarruscag mentioned that it's somewhat difficult to break apart the "config" blob. I have a proposal that could work to break apart things. Some config options, like those related to grid movement or timestepping or nondimensionalization, are used throughout a lot of the code. It will be difficult to make headway there. But some config settings have a very localized impact in the code. To find examples, just look for common prefixes in the config_template.cfg. There's DV_*, FFD_*, UQ_*, INC_*, etc.

We could break apart the config class into a group of classes. Each class would derive from the base config class and share its features. Any function that needs config settings could then pull in two config objects, one with global settings (like nondimensionalization) and one with problem-specific settings (like incompressible flow settings). That way, changing the problem-specific settings, such as the incompressible flow options, won't trigger a complete rebuild of the code. It would only trigger a rebuild of the code that uses the incompressible flow options.

That's my idea. Aside from that idea, I agree with the changes as proposed by @economon.

[pcarruscag]

@economon should we have one file per logical group with all the required includes, so that elsewhere you can still write "#include numerics" instead of having to include all one by one?

@clarkpede that would require passing a "matrix" of CConfigs like we do for numerics and so on right? Which would mean making things virtual and so the inlines would not be inline anymore (I think). I like the idea of breaking up the blob... but.

[clarkpede]

@pcarruscag Yes, we would have a group (or matrix, I guess) of CConfigs. We don't have to follow a rigorous "one-config-per-class" definition; we could make a lot of headway just by splitting things up very roughly.

If I understand, you're thinking we would have to have a base class with lots of virtual functions, and then derived classes with specific implementations. I don't think that would be necessary. First off, that wouldn't solve any problems, since the base class would have to have virtual placeholders for all the config classes. Second, the respective solver classes know what calls they need to make. So they can just have a specific derived class as a parameter (e.g. CIncSolverConfig). They don't need an abstract base class because we expect the incompressible settings to incompressible settings, and we won't ever switch it out for any other type of settings. The derived classes (e.g. CIncSolverConfig) could implement their own config getters/setters, without having any matching virtual functions in the base Config class. That should allow us to keep our inlines and avoid dynamic dispatches.

There may be other flaws with my thinking that i'm not seeing. I just wanted to put my idea out there so we can make more informed decisions. I can also move this discussion to a separate issue, if we want to flesh out this discussion more.