Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

name change inject_astroidwind to inject_randomwind #566

Merged
merged 7 commits into from
Aug 5, 2024

Conversation

moverton000
Copy link
Contributor

Type of PR:
modification to existing code

Description:
The inject_asteroidwind file has been changed to no longer use the term "asteroid" since it can be applied to general object with winds. Used as a pull request demo for the phantom users conference 2024.

Testing:
Phantom was compiled successfully.

Did you run the bots? no

Did you update relevant documentation in the docs directory? no

@danieljprice
Copy link
Owner

danieljprice commented Jul 30, 2024

Hi Madeleine, a bunch of tests were failing because of a network issue, so I just re-ran those tests which are now passing. The only one left is a "real" failure:

 make SETUP=randomwind checking phantom setup runs, creates .setup and .in files with no unspecified user input
  Checking randomwind (setup)... OK
  compiling phantom with SETUP=randomwind
  phantomsetup exists
  runs
  creates .setup file
  creates .in file
  phantom exists
  FAIL: ./phantom myrun.in fails with error

The test here is that you must be able to compile phantomsetup, run ./phantomsetup myrun, press enter a bunch of times and then run ./phantom myrun.in and have it succeed in replacing the _00000.tmp file with the _00000 file. This is currently failing for some reason with SETUP=randomwind...

Hope that helps!

@moverton000
Copy link
Contributor Author

Hi Daniel, I believe the problem is that SETUP=randomwind uses setup_disc which by default only has one sink, but inject_randomwid requires 2 sink particles. So although they can be used together you have to manually specify 2 sinks in the setup. I don't think the defaults for setup_disc should be changed and since the purpose of this pull request is just to rename inject_asteroidwind.f90 to inject_randomwind.f90, I think the best solution is just to remove the SETUP=randomwind option until inject_randomwind can be updated to handle only one sink particle.

danieljprice added a commit that referenced this pull request Aug 5, 2024
@danieljprice danieljprice merged commit 7ce5055 into danieljprice:master Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants