Refactor Region code to RegionState and RegionConverter

[pford]

Description

• What is implemented or fixed? Mention the linked issue(s), if any.
  Implements Move Region conversions for matched images to a new class #1347

• How does this PR solve the issue? Give a brief summary.

  • Code maintenance: the Region class needed to be streamlined.
    • RegionState struct was moved to a separate file, with some Region functions now in the struct.
    • RegionConverter class contains code (formerly in Region) to convert the region to a non-reference image, either as the same region type or as a polygon approximation, and return it as casacore::LCRegion or casacore::Record.
    • Region is now only for the region in the reference image. It creates a RegionConverter when needed for matched images.
    • Other cleanup: unused methods and includes removed.
  • Testing: additional backend tests were created for reference and matched regions, to validate the refactor and for better code coverage.
  • Performance: to retrieve the Region as LCRegion or a Record, for all images (1) the control points were converted from pixel to world coordinates and used to create a casacore::WCRegion, (2) WCRegion::toLCRegion converts back to pixel coordinates, (3) (if requested) LCRegion::toRecord creates a Record. For the reference image, however, the conversion to world coordinates and back to pixel coordinates is unnecessary and adds time. This especially affects the performance of line spatial profiles and PV image generation where numerous regions are created along the line and their LCRegions are used for the statistics. Instead, for the reference file ID a Record is created from the control points, which can be returned as a Record for export or as LCRegion using LCRegion::fromRecord for analytics.

• Are there any companion PRs (frontend, protobuf)?
  No, backend change only

• Is there anything else that testers should know (e.g. exactly how to reproduce the issue)?
  The behavior should be unchanged by this refactor, except in small underlying details (e.g. LCRegion::toRecord returns the coordinates 1-based but creating the Record from control points is 0-based, as specified in the Record's "oneRel" field).

Checklist

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

[kswang1029]

@pford A number of issues detected via e2e tests:

1. when requesting pv preview, the backend will crash.

Crashed Thread: 2

Exception Type: EXC_CRASH (SIGABRT)
Exception Codes: 0x0000000000000000, 0x0000000000000000

Termination Reason: Namespace SIGNAL, Code 6 Abort trap: 6
Terminating Process: carta_backend [99446]

Application Specific Information:
abort() called

Thread 2 Crashed:
0 libsystem_kernel.dylib 0x1a9718744 __pthread_kill + 8
1 libsystem_pthread.dylib 0x1a974fc28 pthread_kill + 288
2 libsystem_c.dylib 0x1a965dae8 abort + 180
3 libc++abi.dylib 0x1a9708b84 abort_message + 132
4 libc++abi.dylib 0x1a96f83b4 demangling_terminate_handler() + 320
5 libobjc.A.dylib 0x1a93cee68 _objc_terminate() + 160
6 libc++abi.dylib 0x1a9707f48 std::__terminate(void ()()) + 16
7 libc++abi.dylib 0x1a970ad34 __cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception) + 36
8 libc++abi.dylib 0x1a970ace0 __cxa_throw + 140
9 libcasa_casa.7.dylib 0x1029d9898 casacore::ArrayBase::validateConformance(casacore::ArrayBase const&) const + 144
10 libcasa_casa.7.dylib 0x1029d9b7c casacore::ArrayBase::copyVectorHelper(casacore::ArrayBase const&) + 80
11 libcasa_casa.7.dylib 0x102a5ce64 casacore::Vector::assign_conforming_implementation(casacore::Vector const&, std::__1::integral_constant<bool, true>) + 44
12 carta_backend 0x10108f828 casacore::Vector::assign_conforming(casacore::Vector const&) + 12 (Vector.h:181) [inlined]
13 carta_backend 0x10108f828 casacore::Vector::operator=(casacore::Vector const&) + 12 (Vector.h:191) [inlined]
14 carta_backend 0x10108f828 carta::RegionHandler::CalculatePvPreviewImage(int, int, bool, std::__1::shared_ptrcarta::PvPreviewCut, std::__1::shared_ptrcarta::PvPreviewCube, std::__1::function<void (float)>, CARTA::PvResponse&, carta::GeneratedImage&) + 2304 (RegionHandler.cc:1276)
15 carta_backend 0x10108d410 carta::RegionHandler::CalculatePvPreviewImage(int, int, int, AxisRange&, bool, std::__1::shared_ptrcarta::Frame&, CARTA::PvPreviewSettings const&, std::__1::function<void (float)>, CARTA::PvResponse&, carta::GeneratedImage&) + 3164 (RegionHandler.cc:1170)
16 carta_backend 0x10108c620 carta::RegionHandler::CalculatePvImage(CARTA::PvRequest const&, std::__1::shared_ptrcarta::Frame&, std::__1::function<void (float)>, CARTA::PvResponse&, carta::GeneratedImage&) + 1324 (RegionHandler.cc:1010)
17 carta_backend 0x1010c3240 carta::Session::OnPvRequest(CARTA::PvRequest const&, unsigned int) + 992 (Session.cc:1353)
18 carta_backend 0x1010d8a94 carta::GeneralMessageTaskCARTA::PvRequest::execute() + 28 (OnMessageTask.tcc:34)
19 carta_backend 0x101105cdc carta::ThreadManager::StartEventHandlingThreads(int)::$_0::operator()() const + 96 (ThreadingManager.cc:54) [inlined]
20 carta_backend 0x101105cdc decltype(static_castcarta::ThreadManager::StartEventHandlingThreads(int)::$_0(fp)()) std::__1::__invokecarta::ThreadManager::StartEventHandlingThreads(int)::$_0(carta::ThreadManager::StartEventHandlingThreads(int)::$_0&&) + 96 (type_traits:3918) [inlined]
21 carta_backend 0x101105cdc void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_deletestd::__1::__thread_struct>, carta::ThreadManager::StartEventHandlingThreads(int)::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_deletestd::__1::__thread_struct>, carta::ThreadManager::StartEventHandlingThreads(int)::$_0>&, std::__1::__tuple_indices<>) + 96 (thread:287) [inlined]
22 carta_backend 0x101105cdc void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_deletestd::__1::__thread_struct>, carta::ThreadManager::StartEventHandlingThreads(int)::$_0>>(void*) + 192 (thread:298)
23 libsystem_pthread.dylib 0x1a974ffa8 _pthread_start + 148
24 libsystem_pthread.dylib 0x1a974ada0 thread_start + 8

2. when exporting a CRTF region file in world coordinates, the position angle of an ellipse region has a small shift.

dev:
ellipse [[17:56:21.28865, -021.57.23.2793], [2.5532arcsec, 5.1064arcsec], 0.00000000deg] coord=J2000, corr=[I], linewidth=2, linestyle=-, symsize=1, symthick=1, color=2ee6d6, font=Helvetica, fontsize=10, fontstyle=bold, usetex=false
this_branch:
ellipse [[17:56:21.28865, -021.57.23.2793], [2.5532arcsec, 5.1064arcsec], 0.00000250deg] coord=J2000, corr=[I], linewidth=2, linestyle=-, symsize=1, symthick=1, color=2ee6d6, font=Helvetica, fontsize=10, fontstyle=bold, usetex=false

3. when exporting annotation as files (regardless it is casa/ds9 or world/pixel), the coordinates have shifts. Take CRTF in pixel coordinates as an example:

dev:
#CRTFv0 CASA Region Text Format version 0
ann symbol [[60.7766pix, 350.8192pix], s] coord=J2000, linewidth=2, linestyle=-, color=ffba01, symsize=6, symthick=1
ann line [[162.9043pix, 146.5638pix], [265.0319pix, 248.6915pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann centerbox [[213.9681pix, 197.6277pix], [163.4043pix, 122.5532pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann ellipse [[213.9681pix, 197.6277pix], [81.7021pix, 51.0638pix], 0deg] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann poly [[101.6277pix, 309.9681pix], [336.5213pix, 212.9468pix], [213.9681pix, 197.6277pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann polyline [[326.3085pix, 85.2872pix], [152.6915pix, 121.0319pix], [213.9681pix, 197.6277pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
# compass [[293.6277pix, 289.5425pix], 100.0000pix] coord=J2000, linewidth=2, linestyle=-, color=ffba01, font=Helvetica, fontsize=20, fontstyle=normal, usetex=false compass=J2000 {N} {E} 1 1
# ruler [[213.9681pix, 197.6277pix], [122.0532pix, 151.6702pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01, font=Helvetica, fontsize=13, fontstyle=normal, usetex=false ruler=J2000 degrees
this_branch:
#CRTFv0 CASA Region Text Format version 0
ann symbol [[59.2995pix, 352.1110pix], s] coord=J2000, linewidth=2, linestyle=-, color=ffba01, symsize=6, symthick=1
ann line [[162.4021pix, 145.9057pix], [265.5048pix, 249.0083pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann centerbox [[213.9535pix, 197.4570pix], [164.9642pix, 123.7232pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann ellipse [[213.9535pix, 197.4570pix], [82.4821pix, 51.5513pix], 0deg] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann poly [[100.5406pix, 310.8699pix], [337.6766pix, 212.9224pix], [213.9535pix, 197.4570pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
ann polyline [[327.3664pix, 84.0442pix], [152.0919pix, 120.1301pix], [213.9535pix, 197.4570pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01
# compass [[294.3735pix, 290.2494pix], 100.0000pix] coord=J2000, linewidth=2, linestyle=-, color=ffba01, font=Helvetica, fontsize=20, fontstyle=normal, usetex=false compass=J2000 {N} {E} 1 1
# ruler [[213.9535pix, 197.4570pix], [121.1611pix, 151.0609pix]] coord=J2000, linewidth=2, linestyle=-, color=ffba01, font=Helvetica, fontsize=13, fontstyle=normal, usetex=false ruler=J2000 degrees

4. When requesting a region spectral profile from a point region, the backend shows an error as

[2024-01-31 01:29:34.727Z] [CARTA] [error] Error applying region 1 to file 0: ?
LCIntersection::LCIntersection - regions do not overlap

and results an empty profile plot.

[kswang1029]

@pford Thanks for addressing the issues I found. All the four issues are fixed.
There is one last thing:
when exporting an annotation ellipse, the position angle seems having a small offset.
this branch:
ann ellipse [[17:56:21.28865, -021.57.23.2793], [4.0851arcsec, 2.5532arcsec], 0.00000250deg] coord=J2000, corr=[I], linewidth=2, linestyle=-, symsize=1, symthick=1, color=ffba01, font=Helvetica, fontsize=10, fontstyle=bold, usetex=false

dev:
ann ellipse [[17:56:21.28865, -021.57.23.2793], [4.0851arcsec, 2.5532arcsec], 0.00000000deg] coord=J2000, corr=[I], linewidth=2, linestyle=-, symsize=1, symthick=1, color=ffba01, font=Helvetica, fontsize=10, fontstyle=bold, usetex=false

[pford]

@kswang1029 I tested this branch pam/1347_region_converter and the combined branch pam/combine_1347_1339_branches on Ubuntu and macOS 13.6, exporting ann ellipse to CRTF World in the reference image and a matched image. In all cases, the angle was 0.00000000deg.

[kswang1029]

@kswang1029 I tested this branch pam/1347_region_converter and the combined branch pam/combine_1347_1339_branches on Ubuntu and macOS 13.6, exporting ann ellipse to CRTF World in the reference image and a matched image. In all cases, the angle was 0.00000000deg.

Odd. I will do some extra multi-platform tests.

[kswang1029]

@pford I tried the following platforms but got exactly the same results (thus test failure):

• apple silicon - Ventura
• intel - Monterey
• intel - Ubuntu 22.04

What's your test platform? Any relevant libraries that we should compare their versions?

meanwhile I will check test workflow.

[kswang1029]

@pford I spotted a behavior about the PA of ellipse and ann ellipse. When we make it as "oblate", the PA is 0 deg. However, if we make it as "prolate", there will be a small shift in PA.

Could you see if you can reproduce this in your env please?

I confirm that with v4.1 PA is still 0deg.

[pford]

@kswang1029 fixed by using double instead of float for angle in ellipse record. More accurate when converted from rad to deg.

[acdo2002]

This PR has been passed for all the ICD-RxJS and performance tests. No regression has been found.
The performance of matched images PV request has been improved.
Following is the result of this PR branch:

PASS src/test/PV_GENERATOR_MATCH_SPATIAL.test.ts (5.026 s)
PV_GENERATOR_MATCH_SPATIAL:Testing PV generator with two spatially matched images.
Register a session
✓ check connection (1 ms)
✓ Get basepath and modify the directory path (3 ms)
Go to "set_QA" folder and open "HD163296_CO_2_1.fits" & "HD163296_CO_2_1.image"
✓ (step 1): Open the first image (80 ms)
✓ (step 2): set cursor and add required tiles to the first image (2 ms)
✓ (step 3): Open the second image (18 ms)
✓ (step 4): set cursor and add required tiles to the second image (2 ms)
✓ (Step 5): set SET_REGION to the first image (1 ms)
✓ (step 6): Match the first image to the second image (149 ms)
✓ (Step 7): 1st image PV Request (2264 ms)
✓ (Step 7): 2nd image PV Request (532 ms)

Test Suites: 1 passed, 1 total
Tests: 10 passed, 10 total
Snapshots: 0 total
Time: 5.051 s

(step 6): Match the first image to the second image (890 ms of dev branch -> 149 ms of this PR)
=> 6x faster
(Step 7): 2nd image PV Request (1274 ms of dev branch -> 532 ms of this PR)
=> 2x faster

[kswang1029]

Looks good. No regression spotted from e2e tests. 👍

[github-actions]

Package	Line Rate	Health
src.Cache	72%	➖
src.DataStream	44%	➖
src.FileList	67%	➖
src.Frame	36%	❌
src.HttpServer	42%	➖
src.ImageData	28%	❌
src.ImageFitter	83%	✔
src.ImageGenerators	43%	➖
src.ImageStats	75%	✔
src.Logger	37%	❌
src.Main	52%	➖
src.Region	69%	➖
src.Session	4%	❌
src.Table	52%	➖
src.ThreadingManager	67%	➖
src.Timer	85%	✔
src.Util	40%	➖
Summary	46% (8589 / 18738)	➖