Merge branch 'dev' of https://github.com/CARTAvis/carta-backend into …

…mark/fix_the_warning_from_Xcode

.github/pull_request_template.md

@@ -13,4 +13,4 @@
 - [ ] 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
+- [ ] GitHub Project estimate added

CHANGELOG.md

@@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Remove program settings equality operators ([#1001](https://github.com/CARTAvis/carta-backend/issues/1001)).
 * Normalize the style of guard names in header files ([#1023](https://github.com/CARTAvis/carta-backend/issues/1023)).
 * Improved file IDs for generated images ([#1224](https://github.com/CARTAvis/carta-frontend/issues/1224)).
+* Store global settings in a singleton class ([#1302](https://github.com/CARTAvis/carta-backend/issues/1302)).
+* Move Region code to RegionConverter for maintenance and performance ([#1347](https://github.com/CARTAvis/carta-backend/issues/1347)).
+* Improve performance of region spatial profiles and PV image generation ([#1339](https://github.com/CARTAvis/carta-backend/issues/1339)).
 
 ## [4.1.0]
 

CMakeLists.txt

@@ -109,6 +109,17 @@ endif ()
 FIND_PACKAGE(HDF5 REQUIRED COMPONENTS CXX)
 FIND_PACKAGE(Protobuf REQUIRED)
 INCLUDE_DIRECTORIES(${PROTOBUF_INCLUDE_DIR})
+
+if (Protobuf_VERSION VERSION_GREATER_EQUAL "4.25.3")
+    FIND_PACKAGE(absl REQUIRED)
+    set(PROTOBUF_LIBRARY
+            ${PROTOBUF_LIBRARY}
+            absl_hash
+            absl_log_internal_message
+            absl_log_internal_nullguard)
+    message(STATUS "Newer protobuf version (${Protobuf_VERSION}) includes abseil libraries")
+endif ()
+
 FIND_PACKAGE(Threads)
 INCLUDE_DIRECTORIES(${HDF5_INCLUDE_DIR})
 
@@ -233,6 +244,7 @@ set(SOURCE_FILES
         src/Region/Ds9ImportExport.cc
         src/Region/LineBoxRegions.cc
         src/Region/Region.cc
+        src/Region/RegionConverter.cc
         src/Region/RegionHandler.cc
         src/Region/RegionImportExport.cc
         src/Session/CursorSettings.cc

src/Frame/Frame.cc

@@ -182,6 +182,11 @@ int Frame::StokesAxis() {
     return _stokes_axis;
 }
 
+bool Frame::IsCurrentZStokes(const StokesSource& stokes_source) {
+    return (stokes_source.z_range.from == stokes_source.z_range.to) && (stokes_source.z_range.from == CurrentZ()) &&
+           (stokes_source.stokes == CurrentStokes());
+}
+
 bool Frame::GetBeams(std::vector<CARTA::Beam>& beams) {
     std::string error;
     bool beams_ok = _loader->GetBeams(beams, error);
@@ -331,12 +336,12 @@ bool Frame::SetImageChannels(int new_z, int new_stokes, std::string& message) {
             bool z_ok(CheckZ(new_z));
             bool stokes_ok(CheckStokes(new_stokes));
             if (z_ok && stokes_ok) {
-                _z_index = new_z;
-                _stokes_index = new_stokes;
-
                 // invalidate the image cache
                 InvalidateImageCache();
 
+                _z_index = new_z;
+                _stokes_index = new_stokes;
+
                 if (!(_loader->UseTileCache() && _loader->HasMip(2)) || IsComputedStokes(_stokes_index)) {
                     // Reload the full channel cache for loaders which use it
                     FillImageCache();
@@ -366,7 +371,6 @@ bool Frame::SetCursor(float x, float y) {
 
 bool Frame::FillImageCache() {
     // get image data for z, stokes
-
     bool write_lock(true);
     queuing_rw_mutex_scoped cache_lock(&_cache_mutex, write_lock);
 
@@ -1605,68 +1609,120 @@ bool Frame::GetSlicerSubImage(const StokesSlicer& stokes_slicer, casacore::SubIm
 bool Frame::GetRegionData(const StokesRegion& stokes_region, std::vector<float>& data, bool report_performance) {
     // Get image data with a region applied
     Timer t;
-    casacore::SubImage<float> sub_image;
-    bool subimage_ok = GetRegionSubImage(stokes_region, sub_image);
+    std::vector<bool> region_mask;
 
-    if (!subimage_ok) {
-        return false;
-    }
-
-    casacore::IPosition subimage_shape = sub_image.shape();
-    if (subimage_shape.empty()) {
-        return false;
+    if (IsCurrentZStokes(stokes_region.stokes_source)) {
+        try {
+            // Slice cached image data using LCRegion bounding box
+            casacore::Slicer bounding_box = stokes_region.image_region.asLCRegion().boundingBox();
+            StokesSlicer stokes_slicer(stokes_region.stokes_source, bounding_box);
+            data.resize(bounding_box.length().product());
+
+            if (GetSlicerData(stokes_slicer, data.data())) {
+                // Next get the LCRegion as a mask (LCRegion is a Lattice<bool>)
+                casacore::Array<bool> tmpmask = stokes_region.image_region.asLCRegion().get();
+                region_mask = tmpmask.tovector();
+            } else {
+                data.clear();
+            }
+        } catch (const casacore::AipsError& err) {
+            // ImageRegion underlying region was not LCRegion
+            data.clear();
+        }
     }
 
-    try {
-        casacore::IPosition start(subimage_shape.size(), 0);
-        casacore::IPosition count(subimage_shape);
-        casacore::Slicer slicer(start, count); // entire subimage
-        bool is_computed_stokes(!stokes_region.stokes_source.IsOriginalImage());
+    if (data.empty()) {
+        // Apply region to image to get SubImage data
+        casacore::SubImage<float> sub_image;
+        bool subimage_ok = GetRegionSubImage(stokes_region, sub_image);
 
-        // Get image data
-        std::unique_lock<std::mutex> ulock(_image_mutex);
-        if (_loader->IsGenerated() || is_computed_stokes) { // For the image in memory
-            casacore::Array<float> tmp;
-            sub_image.doGetSlice(tmp, slicer);
-            data = tmp.tovector();
-        } else {
-            data.resize(subimage_shape.product()); // must size correctly before sharing
-            casacore::Array<float> tmp(subimage_shape, data.data(), casacore::StorageInitPolicy::SHARE);
-            sub_image.doGetSlice(tmp, slicer);
+        if (!subimage_ok) {
+            return false;
         }
 
-        // Get mask that defines region in subimage bounding box
-        casacore::Array<bool> tmpmask;
-        sub_image.doGetMaskSlice(tmpmask, slicer);
-        ulock.unlock();
+        casacore::IPosition subimage_shape = sub_image.shape();
+        if (subimage_shape.empty()) {
+            return false;
+        }
 
-        // Apply mask to data
-        std::vector<bool> datamask = tmpmask.tovector();
-        for (size_t i = 0; i < data.size(); ++i) {
-            if (!datamask[i]) {
-                data[i] = NAN;
+        try {
+            casacore::IPosition start(subimage_shape.size(), 0);
+            casacore::IPosition count(subimage_shape);
+            casacore::Slicer slicer(start, count); // entire subimage
+            bool is_computed_stokes(!stokes_region.stokes_source.IsOriginalImage());
+
+            // Get image data and mask, with image mutex locked
+            std::unique_lock<std::mutex> ulock(_image_mutex);
+            casacore::Array<float> tmpdata;
+            if (_loader->IsGenerated() || is_computed_stokes) { // For the image in memory
+                sub_image.doGetSlice(tmpdata, slicer);
+                data = tmpdata.tovector();
+            } else {
+                data.resize(subimage_shape.product()); // must size correctly before sharing
+                tmpdata = casacore::Array<float>(subimage_shape, data.data(), casacore::StorageInitPolicy::SHARE);
+                sub_image.doGetSlice(tmpdata, slicer);
             }
+
+            // Get mask that defines region in subimage bounding box
+            casacore::Array<bool> tmpmask;
+            sub_image.doGetMaskSlice(tmpmask, slicer);
+            ulock.unlock();
+            region_mask = tmpmask.tovector();
+        } catch (const casacore::AipsError& err) {
+            data.clear();
+            return false;
         }
+    }
 
-        if (report_performance) {
-            spdlog::performance("Get region subimage data in {:.3f} ms", t.Elapsed().ms());
+    // Apply mask to data
+    for (size_t i = 0; i < data.size(); ++i) {
+        if (!region_mask[i]) {
+            data[i] = NAN;
         }
+    }
 
-        return true;
-    } catch (casacore::AipsError& err) {
-        data.clear();
+    if (report_performance) {
+        spdlog::performance("Get region subimage data in {:.3f} ms", t.Elapsed().ms());
     }
 
-    return false;
+    return true;
 }
 
 bool Frame::GetSlicerData(const StokesSlicer& stokes_slicer, float* data) {
-    // Get image data with a slicer applied
+    // Get image data with a slicer applied; data must be correctly resized
+    bool data_ok(false);
     casacore::Array<float> tmp(stokes_slicer.slicer.length(), data, casacore::StorageInitPolicy::SHARE);
-    std::unique_lock<std::mutex> ulock(_image_mutex);
-    bool data_ok = _loader->GetSlice(tmp, stokes_slicer);
-    _loader->CloseImageIfUpdated();
-    ulock.unlock();
+
+    if (_image_cache_valid && IsCurrentZStokes(stokes_slicer.stokes_source)) {
+        // Slice image cache
+        auto cache_shape = ImageShape();
+        auto slicer_start = stokes_slicer.slicer.start();
+        auto slicer_end = stokes_slicer.slicer.end();
+
+        // Adjust cache shape and slicer for single channel and stokes
+        if (_spectral_axis >= 0) {
+            cache_shape(_spectral_axis) = 1;
+            slicer_start(_spectral_axis) = 0;
+            slicer_end(_spectral_axis) = 0;
+        }
+        if (_stokes_axis >= 0) {
+            cache_shape(_stokes_axis) = 1;
+            slicer_start(_stokes_axis) = 0;
+            slicer_end(_stokes_axis) = 0;
+        }
+        casacore::Slicer cache_slicer(slicer_start, slicer_end, casacore::Slicer::endIsLast);
+
+        queuing_rw_mutex_scoped cache_lock(&_cache_mutex, false); // read lock
+        casacore::Array<float> image_cache_as_array(cache_shape, _image_cache.get(), casacore::StorageInitPolicy::SHARE);
+        tmp = image_cache_as_array(cache_slicer);
+        data_ok = true;
+    } else {
+        // Use loader to slice image
+        std::unique_lock<std::mutex> ulock(_image_mutex);
+        data_ok = _loader->GetSlice(tmp, stokes_slicer);
+        _loader->CloseImageIfUpdated();
+        ulock.unlock();
+    }
     return data_ok;
 }
 

src/Frame/Frame.h

@@ -105,6 +105,7 @@ class Frame {
     size_t NumStokes(); // if no stokes axis, nstokes=1
     int CurrentZ();
     int CurrentStokes();
+    bool IsCurrentZStokes(const StokesSource& stokes_source);
     int SpectralAxis();
     int StokesAxis();
     bool GetBeams(std::vector<CARTA::Beam>& beams);
@@ -296,7 +297,6 @@ class Frame {
     ContourSettings _contour_settings;
 
     // Image data cache and mutex
-    //    std::vector<float> _image_cache; // image data for current z, stokes
     long long int _image_cache_size;
     std::unique_ptr<float[]> _image_cache;
     bool _image_cache_valid;       // cached image data is valid for current z and stokes

src/ImageGenerators/PvPreviewCube.cc

@@ -12,6 +12,7 @@
 #include <casacore/images/Images/RebinImage.h>
 
 #include "DataStream/Smoothing.h"
+#include "Logger/Logger.h"
 #include "Timer/Timer.h"
 
 #define LOAD_DATA_PROGRESS_INTERVAL 1000
@@ -162,8 +163,8 @@ bool PvPreviewCube::GetRegionProfile(const casacore::Slicer& region_bounding_box
     auto box_start = region_bounding_box.start();
     auto box_length = region_bounding_box.length();
 
-    // Initialize profile
-    size_t nchan = box_length(_preview_image->coordinates().spectralAxisNumber());
+    // Initialize profile to channels in preview image
+    size_t nchan = _preview_image->shape()(_preview_image->coordinates().spectralAxisNumber());
     profile.resize(nchan, NAN);
     std::vector<double> npix_per_chan(nchan, 0.0);
     auto data_shape = _cube_data.shape();

src/ImageGenerators/PvPreviewCube.h

@@ -7,13 +7,13 @@
 #ifndef CARTA_SRC_IMAGEGENERATORS_PVPREVIEWCUBE_H_
 #define CARTA_SRC_IMAGEGENERATORS_PVPREVIEWCUBE_H_
 
+#include <casacore/images/Images/SubImage.h>
+
 #include "ImageGenerators/ImageGenerator.h"
 #include "Region/Region.h"
 #include "Util/File.h"
 #include "Util/Image.h"
 
-#include <casacore/images/Images/SubImage.h>
-
 namespace carta {
 
 struct PreviewCubeParameters {

src/ImageGenerators/PvPreviewCut.h

@@ -7,6 +7,8 @@
 #ifndef CARTA_SRC_IMAGEGENERATORS_PVPREVIEWCUT_H_
 #define CARTA_SRC_IMAGEGENERATORS_PVPREVIEWCUT_H_
 
+#include <carta-protobuf/pv_preview.pb.h>
+
 #include "Region/Region.h"
 #include "Util/File.h"
 

src/Logger/Logger.cc

@@ -6,15 +6,23 @@
 
 #include "Logger.h"
 
+#include "Main/ProgramSettings.h"
+
 #include <string>
 
 namespace carta {
 namespace logger {
 
 static bool log_protocol_messages(false);
 
-void InitLogger(bool no_log_file, int verbosity, bool log_performance, bool log_protocol_messages_, fs::path user_directory) {
-    log_protocol_messages = log_protocol_messages_;
+void InitLogger() {
+    // Copy parameters from the global settings
+    auto& settings = ProgramSettings::GetInstance();
+    log_protocol_messages = settings.log_protocol_messages;
+    auto no_log = settings.no_log;
+    auto user_directory = settings.user_directory;
+    auto verbosity = settings.verbosity;
+    auto log_performance = settings.log_performance;
 
     // Set the stdout/stderr console
     auto console_sink = std::make_shared<spdlog::sinks::carta_sink>();
@@ -26,7 +34,7 @@ void InitLogger(bool no_log_file, int verbosity, bool log_performance, bool log_
 
     // Set a log file with its full name, maximum size and the number of rotated files
     std::string log_fullname;
-    if (!no_log_file) {
+    if (!no_log) {
         log_fullname = (user_directory / "log/carta.log").string();
         auto stdout_log_file_sink = std::make_shared<spdlog::sinks::rotating_file_sink_mt>(log_fullname, LOG_FILE_SIZE, ROTATED_LOG_FILES);
         stdout_log_file_sink->set_formatter(
@@ -72,7 +80,7 @@ void InitLogger(bool no_log_file, int verbosity, bool log_performance, bool log_
     // Set as the default logger
     spdlog::set_default_logger(default_logger);
 
-    if (!no_log_file) {
+    if (!no_log) {
         spdlog::info("Writing to the log file: {}", log_fullname);
     }
 
@@ -87,7 +95,7 @@ void InitLogger(bool no_log_file, int verbosity, bool log_performance, bool log_
 
         // Set a log file with its full name, maximum size and the number of rotated files
         std::string perf_log_fullname;
-        if (!no_log_file) {
+        if (!no_log) {
             perf_log_fullname = (user_directory / "log/performance.log").string();
             auto perf_log_file_sink =
                 std::make_shared<spdlog::sinks::rotating_file_sink_mt>(perf_log_fullname, LOG_FILE_SIZE, ROTATED_LOG_FILES);

src/Logger/Logger.h

@@ -110,7 +110,7 @@ class carta_sink : public ansicolor_sink<details::console_mutex> {
 
 namespace carta {
 namespace logger {
-void InitLogger(bool no_log_file, int verbosity, bool log_performance, bool log_protocol_messages_, fs::path user_directory);
+void InitLogger();
 void LogReceivedEventType(const CARTA::EventType& event_type);
 void LogSentEventType(const CARTA::EventType& event_type);
 void FlushLogFile();

src/Main/Main.cc

@@ -46,14 +46,13 @@ int main(int argc, char* argv[]) {
         sigaction(SIGINT, &sig_handler, nullptr);
 
         // Main
-        carta::ProgramSettings settings(argc, argv);
+        auto settings = ProgramSettings::Initialise(argc, argv);
 
         if (settings.help || settings.version) {
             exit(0);
         }
 
-        carta::logger::InitLogger(
-            settings.no_log, settings.verbosity, settings.log_performance, settings.log_protocol_messages, settings.user_directory);
+        carta::logger::InitLogger();
         settings.FlushMessages(); // flush log messages produced during Program Settings setup
 
         // Send casacore log messages (global and local) to sink.