xpsi-group / xpsi

X-PSI: X-ray Pulse Simulation and Inference
Other
34 stars 20 forks source link

CustomInstrument script in module generator #298

Closed sguillot closed 1 year ago

sguillot commented 1 year ago

I noticed that the attenuation in the CustomInterstellar.py script located with the module generator is calculated with:

return self._interpolate(energies)**(self['neutral_hydrogen_column_density'])

But all the CustomInterstellar.py used elsewhere for the runs have:

return self._interpolate(energies)**(self['column_density']/0.4)

with the factor of 0.4.

thjsal commented 1 year ago

I think we discussed this with @DevarshiChoudhury at some point and found that the column density variable (with a slightly different name now) is just defined differently in the modules generated by the module generator (and therefore the difference). But it should work correctly.

sguillot commented 1 year ago

I am not very convinced by your response :-) I haven't noticed any other differences in the CustomInterstellar scripts. And presumably, users will use the interstellar_phot_frac.txt files that are in the Zenodo (I don't think it's distributed with XPSI), which then will give different results no?

DevarshiChoudhury commented 1 year ago

https://github.com/xpsi-group/xpsi/blob/e5a1b2b53043beb8861487349b78f0c4ae3e7588/xpsi/module_generator.py#L848

That correction isn't required since the scaling will simply account for that difference in definition, just that now the N_H value inferred will be in terms of the fiducial column density, so instead of e20 cm^-2, the units will be 0.4e20 cm^-2. This fact is documented in the docstrings generated for N_H linked above. There is no difference as such in the scripts

sguillot commented 1 year ago

I see. Thanks for the clarification. This is going to confuse the hell out of people I think. But I guess, we can close the issue

DevarshiChoudhury commented 1 year ago

The reason behind this choice was that the module generator can be used for files different from interstellar_phot_frac.txt currently being used by us, where the fiducial column density is different.