search

gandalfcode / gandalf

GANDALF (Graphical Astrophysics code for N-body Dynamics And Lagrangian Fluids)
GNU General Public License v2.0
44 stars 12 forks source link

Post initial conditions setup #119

Open rbooth200 opened 7 years ago

rbooth200 commented 7 years ago

These functions are a mess in both SPH and the meshless and are in serious need of refactoring.

giovanni-rosotti commented 7 years ago

Is it real refactoring that they need, or just more comments to explain what we are doing there? If we need to call some function, I don't see how much refactoring we can do - we just have to call those functions. At most what we can do is breaking up the mess in separate multiple functions rather than having a single monolithic block...

rbooth200 commented 7 years ago

Definitely more comments, but I'm not convinced that we are doing all the necessary work and that all the work we are doing is necessary.

Maybe it is ok, but I think we need to review it carefully.

giovanni-rosotti commented 7 years ago

Is this still relevant? I thought some refactoring of the function did happen already

rbooth200 commented 7 years ago

Lots of refactoring has happened. I think what we have is (probably) correct, but I still think its one of the ugliest parts of the code.

I'd be happy to replace this issue with one about more general refactoring, which should include things like UpdateAllProperties etc, for which the code is now almost identical in the Meshless and GradhSph.

giovanni-rosotti commented 7 years ago

Would you say this is still something to go in milestone 1?

rbooth200 commented 7 years ago

I'm happy for this to be milestone 2, but perhaps a priority there. Bug fixes etc are more important in the meantime.