Skip to content

Adding an interface to use DustPy with MCFOST#47

Open
LimitelessAB wants to merge 14 commits into
cpinte:masterfrom
LimitelessAB:master
Open

Adding an interface to use DustPy with MCFOST#47
LimitelessAB wants to merge 14 commits into
cpinte:masterfrom
LimitelessAB:master

Conversation

@LimitelessAB

Copy link
Copy Markdown

Changes in this pull request:

  1. DustPy is a dust evolution code, that calculates amongst other things the density of dust grain along the radial direction. Because these simulations are highly configurable, it is interesting to see the changes this produces in artificial images like the ones MCFOST can create.
    As such, this python package acts as an interface between the two codes to convert DustPy data files into 2D (r and Z direction) density files MCFOST can read via it's -density_file option.
    It leaves the choice of vertical extension of the radial density profile to the user, following the 4 prescriptions MCFOST uses: no settling, parametric settling, Dubrulle settling, or Fromang settling.
    It also provides a tool to control that MCFOST reads correctly its density file.

  2. The init file has been updated to include this new package

  3. The documentation has been updated to explain how to use this new package

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

Copy link
Copy Markdown

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 a new DustPy_integration module to pymcfost, enabling the conversion of DustPy simulation results into MCFOST-compatible FITS density files across various settling models (no settling, parametric, Dubrulle, and Fromang). Feedback highlights several critical issues, including an invalid module import extension in __init__.py, hardcoded loop ranges that ignore function arguments, and an undefined variable in a plotting function. Additionally, improvements were suggested for dynamic subplot handling and the correction of multiple typos in function names and documentation.

Comment thread src/pymcfost/__init__.py Outdated
from .fitsigma import *
from .chi2_functions import *
from .BO_fitting import *
from .DustPy_integration.py import *

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The import statement incorrectly includes the .py file extension. In Python, modules should be imported by their name without the extension. This will cause an ImportError when the package is loaded.

Suggested change
from .DustPy_integration.py import *
from .DustPy_integration import *

Comment thread src/pymcfost/DustPy_integration.py Outdated

Z_cscl[r_cell]=( Hp_array[r_cell] * 7) / number_of_vertical_cells #Defines the size of one vertical cell by deviding 7 times the local pressure scale height by the number of cells (Why 7 times? To match MCFOST)

for Z_cell in range (1,71):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The loop range is hardcoded to 71 (70 iterations), but the function accepts number_of_vertical_cells as an argument and uses it to initialize the arrays. If number_of_vertical_cells is not 70, this will cause an IndexError (if smaller) or fail to populate the entire grid (if larger). This hardcoded value appears in multiple places throughout the file (lines 283, 384, 398, 491, 505, 595, 615, 625) and should be updated to use the argument.

Suggested change
for Z_cell in range (1,71):
for Z_cell in range(1, number_of_vertical_cells + 1):

Comment thread src/pymcfost/DustPy_integration.py Outdated
Comment thread src/pymcfost/DustPy_integration.py Outdated

plt.figure(figsize=(80,80))
for i in range (0,len(DustPy_file[0,:])):
plt.subplot(10,10,i+1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Using plt.subplot(10, 10, i + 1) will raise a ValueError if the number of radial cells (len(DustPy_file[0, :])) exceeds 100. It is recommended to calculate the grid dimensions dynamically or use a more robust plotting method for large grids.

Comment thread docs/usage.rst Outdated
Comment thread src/pymcfost/DustPy_integration.py
Comment thread src/pymcfost/DustPy_integration.py
Comment thread src/pymcfost/DustPy_integration.py
LimitelessAB and others added 7 commits May 25, 2026 12:14
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
added a missing ')'
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.

1 participant