danieljprice / phantom

Phantom Smoothed Particle Hydrodynamics and Magnetohydrodynamics code
https://phantomsph.github.io
Other
110 stars 244 forks source link

an unpadte to randomwind #578

Closed ShunqH closed 3 months ago

ShunqH commented 4 months ago

Type of PR: modification to existing code (asteroidwind)

Description: Change the asteroidwind to randomwind. Update the unit calculation, now the mdot can be input with units. Add a Gaussian random distribution position. The particles can be injected in a Gaussian distribution in theta.

Testing: I tested the old method 'asteroidwin', and a new 'randomwind', both can inject particles properly.

Did you run the bots? yes

Did you update relevant documentation in the docs directory? no

danieljprice commented 3 months ago

Hi Shunquan,

thanks for sending this through. The "failure" here is that we disallow all compiler warnings on the master branch (by adding the -Werror flag), so you just have to get rid of the warning you get when compiling phantomsetup with SETUP=asteroidwind or SETUP=randomwind and this should pass. Don't worry about the conflicting files, as this was due to another pull request I just merged...

The warning you get is just to do with an imported module variables that are not actually used (umass and utime)

Hope that helps!

Daniel

 Checking asteroidwind (setup)... FAILED
  Error: Unused module variable ‘umass’ which has been explicitly imported at (1) [-Werror=unused-variable]
  Error: Unused module variable ‘utime’ which has been explicitly imported at (1) [-Werror=unused-variable]
  make[1]: *** [Makefile:447: inject_randomwind.o] Error 1
  make: *** [Makefile:16: setup] Error 2
ShunqH commented 3 months ago

Hi Shunquan,

thanks for sending this through. The "failure" here is that we disallow all compiler warnings on the master branch (by adding the -Werror flag), so you just have to get rid of the warning you get when compiling phantomsetup with SETUP=asteroidwind or SETUP=randomwind and this should pass. Don't worry about the conflicting files, as this was due to another pull request I just merged...

The warning you get is just to do with an imported module variables that are not actually used (umass and utime)

Hope that helps!

Daniel

Checking asteroidwind (setup)... FAILED
 Error: Unused module variable ‘umass’ which has been explicitly imported at (1) [-Werror=unused-variable]
 Error: Unused module variable ‘utime’ which has been explicitly imported at (1) [-Werror=unused-variable]
 make[1]: *** [Makefile:447: inject_randomwind.o] Error 1
 make: *** [Makefile:16: setup] Error 2

Hi Daniel,

Thanks! This helps a lot. I made more changes after the last push, which probably fixed Madeline's problem, too. I will push again when I finish.

Shunquan

ShunqH commented 3 months ago

Added wind_type for wind setup (0=asteroidwind, 1=randomwind). If wind_type=1, the following features are activated: inject_pt: the particle number that excites wind. It's now compatible with any number of point masses. r_inject: inject radius with units, e.g. 1*AU, 1e8m. wind_speed_factor: set up the wind speed with a factor based on the Keplerian speed at r_inject.

More options for Gaussian-type wind. We assume the maximum wind comes from the equator, so we set up the wind directions based on the spin axis of the star/planet: delta_theta: the standard deviation for the Gaussian distribution theta: the tilt inclination of the star or planet. If theta=90, the spin axis points to the x direction. phi: the tilt orientation of the star. If theta=90 and phi=90, the spin axis points to the y direction.

danieljprice commented 3 months ago

Hi Shunquan, I fixed the conflicts and everything passed so this is merged to master. Please ensure that you click to update your fork to the latest so there are no ongoing conflicts, as I fixed a few other testing issues in the inject_randomwind.f90 routine (unrelated to the new functionality). Please also check everything still works for you...

ShunqH commented 3 months ago

Big thanks!