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

Yuhsuan/1277 fitting error #1294

Merged
merged 16 commits into from
Sep 1, 2023
Merged

Yuhsuan/1277 fitting error #1294

merged 16 commits into from
Sep 1, 2023

Conversation

YuHsuan-Hwang
Copy link
Contributor

@YuHsuan-Hwang YuHsuan-Hwang commented Aug 14, 2023

Description

  • What is implemented or fixed? Mention the linked issue(s), if any.
  1. Fixed image fitting error too small #1277 by implementing error calculations based on Condon (1997), which is also used by casa imfit.

    • When the beam table is empty, we assume the samples are independent and apply formulas in Condon (1997) section 2 eqn 21.
    • When the beam table is not empty, we assume there is correlated noise and apply formulas in Condon (1997) section 3 eqn 41 and 42.
    • If the beam table is not empty but the beam of the current plane is not found, we apply the first beam.
    • If the background offset is a free parameter during fitting, background offset error is calculated by the covariance matrix.
  2. Changed from unweighted fitting to weighted fitting with the information of image standard deviation.

    • Please note that this will change the iteration number and the fitting results and thus the execution time.
  3. Added calculation of integrated flux in the fitting result. This requires frontend branch Yuhsuan/fitting integrated flux carta-frontend#2229 and protobuf branch Yuhsuan/fitting integrated flux carta-protobuf#88.

    • Applied Condon (1997) section 2 eqn 21 and section 3 eqn 42 for the calculation.
    • Applicable only when image units are exactly "Jy/pixel" or "Jy/beam." The calculated flux unit is Jy.
    • When both amplitude and FWHM sizes of a component are fixed, the integrated flux becomes fixed.
  • How does this PR solve the issue? Give a brief summary.

    • Obtained beam size, image unit, and image std (of the entire image or regions) in Frame.FitImage.
    • Obtained Median Absolute Deviation (MAD) as image std in ImageFitter.SolveSystem.
    • Applied weighted fitting (requires image std) in ImageFitter.SolveSystem.
    • Added error calculation (requires beam size and image std) and integrated flux calculation (requires image unit) in ImageFitter.CalculateErrors.
  • Are there any companion PRs (frontend, protobuf)?

  • Is there anything else that testers should know (e.g. exactly how to reproduce the issue)?

  • ToDo

  • code maintenance: add documentation in ImageFitter.h.

Checklist

  • changelog updated / no changelog update needed
  • e2e test passing / added corresponding fix
  • protobuf updated to the latest dev commit / no protobuf update needed
  • added reviewers and assignee
  • added ZenHub estimate, milestone, and release

src/Frame/Frame.cc Outdated Show resolved Hide resolved
@YuHsuan-Hwang
Copy link
Contributor Author

YuHsuan-Hwang commented Aug 22, 2023

New changes:

  • Added documentation in ImageFitter.h.
  • Replaced std of the image / region with Median Absolute Deviation (MAD) calculation for weighted fitting and error calculation.
  • Fixed incorrect center error calculation.

@kswang1029
Copy link
Contributor

this is looking good. I have adjusted e2e tests accordingly.

pa_err = sqrt(4.0 * tmp * tmp / rho_square_3) * 180.0 / M_PI;

if (_unit == "Jy/beam") {
const double flux = 2 * M_PI * fwhm_x * fwhm_y * SQ_FWHM_TO_SIGMA * amp / _beam_size / _beam_size;
Copy link
Contributor

@kswang1029 kswang1029 Sep 1, 2023

Choose a reason for hiding this comment

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

need to divide a solid angle conversion factor as pi / 4.0 / ln(2.0) ~ 1.1330900354567985

flux = 2 * M_PI * fwhm_x * fwhm_y * SQ_FWHM_TO_SIGMA * amp / _beam_size / _beam_size / (M_PI / 4.0 / log(2.0))

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Code Coverage

Package Line Rate Health
src.Cache 66%
src.DataStream 52%
src.FileList 67%
src.Frame 50%
src.HttpServer 42%
src.ImageData 28%
src.ImageFitter 83%
src.ImageGenerators 43%
src.ImageStats 76%
src.Logger 44%
src.Main 54%
src.Region 18%
src.Session 29%
src.Table 52%
src.ThreadingManager 87%
src.Timer 85%
src.Util 48%
Summary 38% (7086 / 18700)

@confluence confluence merged commit e16c26e into dev Sep 1, 2023
14 checks passed
@confluence confluence deleted the yuhsuan/1277_fitting_error branch September 1, 2023 09:35
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.

image fitting error too small
6 participants