Skip to content

Conversation

@lsiess
Copy link
Contributor

@lsiess lsiess commented Oct 13, 2025

Description:

Components modified:

  • Setup (src/setup)
  • Main code (src/main)
  • Moddump utilities (src/utils/moddump)
  • Analysis utilities (src/utils/analysis)
  • Test suite (src/tests)
  • Documentation (docs/)
  • Build/CI (build/ or github actions)

Type of change:

  • Bug fix
  • Physics improvements
  • Better initial conditions
  • Performance improvements
  • Documentation update
  • Better testing
  • Code cleanup / refactor
  • Other (please describe)

Testing:

Did you run the bots? no

Did you update relevant documentation in the docs directory? no

Did you add comments such that the purpose of the code is understandable? yes

Is there a unit test that could be added for this feature/bug? yes

If so, please describe what a unit test might check:
a test should check wind values (rho, velocity) against reference values when running phantom on SETUP=wind with more than one wind emitting sink

Related issues: #

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @lsiess, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly expands the simulation's capabilities in modeling stellar winds by enabling multiple wind-emitting sink particles, each with customizable wind and spin characteristics. It introduces advanced particle injection techniques, such as Fibonacci spheres and specialized methods for jets and rotating winds, to create more accurate initial conditions. Concurrently, the underlying physics has been improved with new high-temperature cooling and heating mechanisms, ensuring a more comprehensive treatment of thermal processes in these dynamic environments.

Highlights

  • Multiple Wind-Emitting Sinks: The simulation now fully supports multiple sink particles emitting stellar winds, with individual wind and spin properties configurable for each sink. This is a major enhancement for modeling complex astrophysical environments.
  • Advanced Wind Injection Methods: New particle injection methods have been introduced, including Fibonacci spheres for quasi-uniform distribution, and specialized routines for polar jets and rotating stellar winds. This allows for more realistic and diverse wind geometries.
  • Improved High-Temperature Cooling Physics: New physical models for high-temperature cooling and heating have been implemented, incorporating contributions from recombination, metal ions, thermal bremsstrahlung, and Compton processes, based on established astrophysical literature.
  • Refactored Wind Setup and Particle Tracking: The entire wind setup and particle injection logic has been refactored to be more modular and sink-specific. Additionally, a new iseed_sink property tracks which sink particle emitted a gas particle, enabling sink-specific radiation pressure calculations.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces significant enhancements and refactoring to support winds from multiple sinks, alongside new physics for high-temperature cooling and more flexible particle injection methods. The overall direction is excellent, improving modularity and physical realism. However, I've identified a few critical issues where the implementation for multiple sinks is not fully generalized and contains hardcoded limitations that could lead to incorrect behavior with more than two sinks. There are also some minor bugs and areas for improvement in the new cooling functions. Addressing these points will greatly improve the robustness and scalability of these new features.

integer, parameter :: nrho_index = 10
integer :: k
real :: inner_radius,r3
integer :: k,isink=1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The sink index isink is hardcoded to 1 within the pulsating_wind_profile subroutine. This will cause incorrect behavior when a pulsating wind is used with any sink other than the first one, as it will use the physical parameters of sink 1.

To fix this, isink should be passed as an argument to this subroutine. The call site in inject_particles is within a loop over sinks, so the isink loop variable is available to be passed.

integer :: k,isink

real :: Mstar_cgs, Lstar_cgs, wind_gamma, Mdot_cgs, Tstar, Rstar
real :: u_to_temperature_ratio
real, dimension (:,:), allocatable, public :: trvurho_1D, JKmuS_1D
real, dimension (:,:), allocatable, target, public :: trvurho_1D,trvurho_1D2,JKmuS_1D
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The implementation for storing 1D wind profiles is limited to a maximum of two sinks by using trvurho_1D and trvurho_1D2. This approach is not scalable for more than two wind-emitting sinks.

To properly support winds from various sinks, this should be generalized. A better approach would be to use a dynamically allocated array of pointers, where each element can point to the wind profile for a specific sink. For example:

type(wind_profile_t), pointer :: trvurho(:) => null()
...
allocate(trvurho(nptmass))

This would make the implementation more robust and scalable.

Comment on lines +194 to +195
! could compute analytical formula but not needed for implicit cooling
dlnQ_dlnT = 0.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Setting dlnQ_dlnT = 0. is acceptable for the implicit cooling solver, as the comment indicates. However, this will lead to inaccuracies if the exact cooling solver (icool_method = 2) is used, as it relies on this derivative. To ensure correctness across all solver options, it would be better to compute the analytical derivative of Q_cgs with respect to T or use a numerical approximation.

Comment on lines 265 to 267
case (3)
state%alpha = state%alpha_Edd+alpha_rad
state%dalpha_dr = (state%alpha_Edd+alpha_rad-alpha_old)/(1.e-10+state%r-state%r_old)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This case (3) block is a duplicate of the one immediately preceding it. This appears to be a copy-paste error and should be removed to avoid confusion.

lsiess and others added 27 commits October 13, 2025 22:15
…ss sink particle arrays inside utility routines; fix issues with bondi injection
…rom desired dr/dp and specified particle mass
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.

4 participants