-
Notifications
You must be signed in to change notification settings - Fork 10
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/150 fov fitting #1128
Yuhsuan/150 fov fitting #1128
Conversation
@YuHsuan-Hwang this draft PR works fine as far as I can tell. It works with matched image as well. @veggiesaurus @confluence @pford please proceed with code review. 🙂 |
Update: added unit tests. also fixed incorrect pixel number in |
pending approval before all e2e test passing again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
via e2e tests, no regression is observed in 4da68be
This PR is for the third item in #150: image fitting with field of view.
Details:
With the fov info sent from the frontend, the backend creates a temporary region to obtain the region data. The image fitter applies an offset to the center positions of Gaussian components when fitting the region. The temporary region is removed after the fitting task is done.
The advantage of this approach is that we support fitting the current fov of a rotated image (usually a spatially matched image) without implementing complicated calculation. In addition, the users can reproduce the fitting result by saving a subimage of the fov region (with info provided in the frontend). The reproduce steps will be simpler after we support image fitting with created regions in the future (probably only few-lines change to support this).
Using the region data requires slightly more memory usage than using the existed image cache, but the patterns of the CPU and momery profiles are similar. Please check the link for details of the performance tests:
https://docs.google.com/document/d/1orLw1Szpy8awVvypkAdAwhkS1u9lYuxkiOah-9hROrE/edit?usp=sharing
Update: also fixed incorrect pixel number in
FuncF()
when fitting images with nan pixelsCorresponding PRs:
CARTAvis/carta-frontend#1885
CARTAvis/carta-protobuf#74
Still working on:add unit testssend protobuf PR