Closed janmees closed 7 years ago
AUTHORS "(2015-1016)" -> "(2016)"
boundary_stl_periodic.h:31: please change
into
to preserve name convention
Is there a Makefile in the repository? If yes, please remove. This should be created by the autotools and not be part of the repository. But Makefile.am should stay!
boundary_stl_periodic.cpp: 164, 167, 179, 182: These lines contain comparisons of doubles representing coordinates. Obviously, with your test-input they worked so far. But could you please perform a "sensitivity" test. I.e., what happens, if there is a slight deviation in the last digit for two supposedly identical coordinates. If necessary create this deviation "by hand" in the STL-file. I fear, for a safe operation, we must introduce an "epsilon". This could be one of the epsilons already existing in the code (should be somewhere in the 'geometry' directory). Or can we expect the user to deliver STL-files where this is absolutely not necessary, by making a statement such as "STL-files produced by CAD software XYZ, are guaranteed to be read by SYMPLER without difficulties."? Then, XYZ would have to be free, non-commercial software. Btw, did you test both ASCII and binary files?
MSG_DEBUG("Boundary Stl Periodic", "Setup completed."); -> MSG_DEBUG("BoundaryStlPeriodic::setup", "Setup completed."); Please check also for all other cases that this format is used.
wall.h: I think your idea was that findcorner(int i) implements default behaviour which is overwritten by child classes. Wouldn't it be better to make the function in wall.h pure virtual? As far as I remember, class Wall is not meant to be instantiated and the code does not do that anywhere. Btw1: Is the comment about extended initializer lists an erroneous copy&paste or do I miss its purpose? Btw2: OK, I admit, now I am becoming really niggling, so I promise that this is the last one: Do you think the name of the function is ideal? How about, e.g., cornerByIndex, returnCorner, returnCornerByIndex,...? The method is not really searching, and hence not really finding, right?
The extended initializer comment is the warning i got while compiling when i used {0,0,0}. Doing it old fashion way x=0; No warning so i changed the way it was implemented since i wasnt sure if sympler is meant to require c++11. Ill look at the rest as soon as possible.
Please add an example to the "examples" folder that does exactly the same thing as https://github.com/kauzlari/sympler/blob/master/examples/SPHperiodicPoiseuille.xml and hence produces exactly the same results. Then we can merge.
lets see if this works