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

Given a convolved image, writeFits does not save values in the units of Jy/beam #19

Open
shengjunlin opened this issue Mar 9, 2022 · 3 comments

Comments

@shengjunlin
Copy link

In lines 373 ~ 376 at radmc3dPy/image.py, radmc3dImage.writeFits() has a wrong unit conversion for images which has been convolved by radmc3dImage.imConv().

        if len(self.fwhm) == 0:
            # Conversion from erg/s/cm/cm/ster to Jy/pixel
            conv = self.sizepix_x * self.sizepix_y / (dpc * nc.pc)**2. * 1e23
        else:
            # If the image has already been convolved with a gaussian psf then self.image has
            # already the unit of erg/s/cm/cm/beam, so we need to multiply it by 10^23 to get
            # to Jy/beam
            conv = 1e23

In the convolved radmc3dImage object returned by imConv(), its self.image still has the unit of erg/s/cm/cm/ster. In fact, this design is consistent with the behavior of image.plotImage(bunit='jy/beam'), which correctly first convert the values in self.image from erg/s/cm/cm/ster to Jy/pixel, and then convert it to Jy/beam in lines 1466 ~1487. This bug in radmc3dImage.writeFits() could be fixed by changing things to

        if len(self.fwhm) == 0:
            # Conversion from erg/s/cm/cm/ster to Jy/pixel
            conv = self.sizepix_x * self.sizepix_y / (dpc * nc.pc)**2. * 1e23
        else:
            # Conversion from erg/s/cm/cm/ster to Jy/beam
            beam_area_sqasec = self.fwhm[0] * self.fwhm[1] * np.pi / 4. / np.log(2.0)
            beam_area_sr = beam_area_sqasec * (np.pi/180./3600.)**2
            conv = beam_area_sr * 1e23
@dullemond
Copy link
Owner

Dear Sheng-Jun Lin,

thanks for pointing this out. This is a Python package that I did not write, so it is hard for me to judge what the author meant with this. But it does seem you are right. I'll keep this issue open and see if someone else (who uses radmc3dPy with the convolution) confirms your suggestion. Then I'll build it in the way you suggest.

Best wishes,
Kees

@shengjunlin
Copy link
Author

Dear Kees,

Thank you for your reply!

Best wishes,
Sheng-Jun

@j6626
Copy link

j6626 commented Mar 2, 2023

Incidentally, I agree with Sheng-Jun Lin's proposed fix.

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

No branches or pull requests

3 participants