Please don't call the elapsed time of a simulation a TimeStep

Dear Development Team,

The following lines are taken from JSph.cpp

double TimeStepIni;     ///<Initial instant of the simulation. | Instante inicial de la simulación.
double TimeStep;        ///<Current instant of the simulation. | Instante actual de la simulación.                                 
double TimeStepM1;      ///<Instant of the simulation when the last PART was stored. | Instante de la simulación en que se grabo el último PART.   

They show that DualSPHysics uses the word TimeStep to define the progress of the simulated time. Time intervals and time instants are two different things, and their difference is crucial in the context of numerical models.

As a consequence, further in the code, one comes across lines such as this from JSphCpuSingle::Run(...):

TimeStep+=stepdt;

where the guest developer has to realise, and keep in mind time and time again, that stepdt is a time step and TimeStepis not a time step.

All the more taxing, this puts one on the lookout that the same semantic traps could lie anywhere else in the code. These oversights are like logs across the road, since the code cannot be read at face value any longer. They defeat the noble purpose of making DSPH accessible to and used by a general community.

Fixing these errors programmatically takes seconds, provided one knows what to touch. Only the original developers have such a command of the entire code structure to do that efficiently and error-free.

Therefore, in the occasion of the release of 5.0 (but also for the current version if you like):

  • I would ask the Development Team of DualSPHysics to take care of a grand revision of the variable and function names, so as to guarantee that word choice and meaning never deviate one from another;
  • Also suggested is to adopt and distribute an endorsed style template for formatting the code consistently with commonly used CUDA-savvy IDEs: this will greatly facilitate merging the contributions from outside developers into the master;
  • Finally, Doxygen files have not been distributed with DualSPHysics for a while. It could be good to offer new developers this classical aid to navigate the intricate class hierarchies and numerous class members.

I tend to assume that the benefits of these actions are self-evident: they encourage (don't discourage) a wider community of practitioners and scholars to take the best out of the distribution of DSPH. Please inform me of any flaw in this assumption.

Thanks for reading this post.

Giordano Lipari

Comments

  • Please create a github fork with your preferred terminology.

    Ben

  • edited May 2020

    Thanks for reading this post @Ben.

    Only the original developers have such a command of the entire code structure to do that efficiently and error-free. I propose not to call the elapsed time a time step for the reasons stated above. If this is an occasion to revise the vocabulary used by DSPH, all the better.

    Making github forks, by the way, will be all the more efficient once a consistent code style for DSPH is adopted and distributed, the second point. In this way guest developers can return a merge with producing a lean diff/patch file. What I did forget to mention here is that the current code style of DSPH is a bit crammed, hard to read and not used consistently. So it is good to leave (guest) developers the possibility to use their preferred (indeed) layout and yet return something formatted according to a standard style. Only difference of content, and not of layout, will stand out in the merge. The attached images express the possibilities behind this idea.

    My two cents.


Sign In or Register to comment.