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

Made dust use dimensional units (where valid) #167

Closed rbooth200 closed 6 years ago

rbooth200 commented 7 years ago

This branch introduces dimensional units into the dust. I've had a nightmare understanding just how to use the classes properly, so I've introduced some helper functions in SimUnits.h that still need implementing. I don't trust myself to do this correctly so David, can you do it? This should only take 2 minutes.

rbooth200 commented 7 years ago

Ok, hopefully this branch will pass, then I'm ready for it to go.

I have a question though: are you happy with the fact that I've introduced a (somewhat arbitrary) constant into the Esptein drag law? The motivation was so that the drag coefficient means something sensible (the area to mass ratio of the grain) for those people who want to use dimensional units. I've updated the user guide accordingly.

dhubber commented 6 years ago

I have a question though: are you happy with the fact that I've introduced a (somewhat arbitrary) constant into the Esptein drag law? The motivation was so that the drag coefficient means something sensible (the area to mass ratio of the grain) for those people who want to use dimensional units. I've updated the user guide accordingly.

Are you referring to this K_D constant which is then scaled to dimensionless units? If so, then that's fine. Having it as a user-defined parameter and then scaling it in a transparent way is how it should be done, rather than just writing K_D = 1.54234... somewhere in the code. There are other instances of doing this in other routines, like the temp0 or rho_bary in the EOS code so this is fine. And it's all self-contained within the dust class so even better.

Btw, this is passing Travis now and the coding seems fine so I'm happy to merge if you're happy it's doing everything correctly also.

rbooth200 commented 6 years ago

I'm happy with it. The only thing I want to check with you guys is that you don't mind that I've changed the meaning of the constant K_D in the Epstein drag law. I think this is an improvement, so as long as you are happy, I'm happy for this branch to go.

dhubber commented 6 years ago

The only thing I want to check with you guys is that you don't mind that I've changed the meaning of the constant K_D in the Epstein drag law.

Oh, I'm not a dust person so I don't give a damn! :-) But as long as it's clear in the comments to other dust people what it is so it's not confusing to them, then I don't mind.

rbooth200 commented 6 years ago

I think the documentation is fine, so there should be no problem.