petercamps
commented
1 year ago @Koseimatsu As a first step, I merged the public master branch into your branch, resolving the merge conflicts by removing the changes you indicated as to be ignored. Below are some comments and further questions on your proposed changes.
-
Why do we need an option to disable the Doppler shift for dust emission? The current behavior is to always include it (just as for other media). Granted, the simulation assumes that all medium components have the same bulk velocity, but even if this assumption would be invalid, the effect for dust is small because the cross sections vary slowly with wavelength. Do you have an example of a simulation where it actually matters? If not, I'd rather not add this option.
-
Once dust convergence has been reached, you skip calculating and logging the dust convergence parameters (essentially, the total dust-absorbed luminosity over all cells). This would effect the simulation results if, in a subsequent iteration, the radiation field (emitted by other primary or secondary components) changes enough to break that dust convergence. This is unlikely to occur in your simulations but I'm not sure this is true in general. Does the logging (and corresponding calculation) really take significant time compared to photon emission? Can you send me (perhaps via regular email) an example of a simulation where this is an issue (preferably including ski file and log file)? As an alternative, perhaps I can look into speeding up this calculation.
-
We previously assumed that the transitions (not the coefficients) are identical for all collision partners. Apparently this assumption is no longer valid and you had to update the code. Does this require a change in any of the existing data files? If so, you need to send them over.
-
Sounds reasonable, although it adds another complication to the user configuration and it is an incompatible change to the ski file. Do you have an example of a simulation where this actually matters, and where your have improved the simulation time and/or the results by specifying a different spatial bias for each medium component?
-
Please send the data files corresponding to these new species.
-
OK.
I look forward to your responses.
Motivation and Guidelines To realize the dust + non-LTE line radiative transfer, I had to make some options in SKIRT, as shown in the commits. （listed in order of importance.)
add an option for considering the Doppler shift of dust emission (Low risk disrupting other classes).
Update MonteCarloSimulation.cpp (This change reduces the computational time. The estimation of the dust temperature takes much time, so it is better to stop updating the dust temperature after it converged. Here, I expect that the iterative update of the dust temperatures is more quickly converged than that of level populations of atoms or molecules, which is typically true.) (medium risk disrupting other classes).
Change the definition of indexes of collision transitions. (Low risk)
Changed spatial bias to be set in EmittigaGasMix and DustEmissionOption (Medium risk disrupting other classes). If we want to see molecular emission lines in the MIR or NIR, dust emission usually comes from diffuse regions although molecular emission comes from dense regions. The individual option will help with this problem.
add OH with hyperfine structures and H2 molecules. level populations of OH in the current SKIRT do not include hyperfine structures. This OH includes hyperfine structures, but the calculation can be more complex. Thus, it is better to make another option.
Debug: for the calculations of mean intensities and solving a matrix (Low risk). This helps to understand what makes errors. Even in public SKIRT, I feel this message is nice.
Unnecessary commits Please ignore these commits.
Tests I have tested how these changes affect other functionalities, and I don't see risks disrupting other classes. but I'm not investigating these things in the entire SKIRT, so I'm not sure. If you feel some risk to these changes, please cancel these requests.