From b0406681c37458ea1c8adc318f259c989182f54d Mon Sep 17 00:00:00 2001 From: Bruce Waddington Date: Tue, 5 Nov 2024 18:31:19 -0800 Subject: [PATCH 1/6] Cleanup of MultiStar processing and UI if subframes enabled --- src/guider_multistar.cpp | 5 +++-- src/star.cpp | 36 +++++++++++++++++++++++------------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/guider_multistar.cpp b/src/guider_multistar.cpp index 075d2399a..a6a7b9684 100644 --- a/src/guider_multistar.cpp +++ b/src/guider_multistar.cpp @@ -466,7 +466,8 @@ bool GuiderMultiStar::AutoSelect(const wxRect& roi) edgeAllowance = wxMax(edgeAllowance, pSecondaryMount->CalibrationTotDistance()); GuideStar newStar; - if (!newStar.AutoFind(*image, edgeAllowance, m_searchRegion, roi, m_guideStars, MAX_LIST_SIZE)) + if (!newStar.AutoFind(*image, edgeAllowance, m_searchRegion, roi, m_guideStars, + (pCamera->UseSubframes || !m_multiStarMode) ? 1 : MAX_LIST_SIZE)) { throw ERROR_INFO("Unable to AutoFind"); } @@ -1213,7 +1214,7 @@ void GuiderMultiStar::OnPaint(wxPaintEvent& event) } // show in-use secondary stars - if (m_multiStarMode && m_guideStars.size() > 1) + if (m_multiStarMode && m_guideStars.size() > 1 && !pCamera->UseSubframes) { if (m_primaryStar.WasFound()) dc.SetPen(wxPen(wxColour(0, 255, 0), 1, wxPENSTYLE_SOLID)); diff --git a/src/star.cpp b/src/star.cpp index 7dc9275f8..472f1d967 100644 --- a/src/star.cpp +++ b/src/star.cpp @@ -1021,22 +1021,26 @@ bool GuideStar::AutoFind(const usImage& image, int extraEdgeAllowance, int searc double minSNR = pFrame->pGuider->GetAFMinStarSNR(); double maxHFD = pFrame->pGuider->GetMaxStarHFD(); foundStars.clear(); - for (std::set::reverse_iterator it = stars.rbegin(); it != stars.rend(); ++it) + if (maxStars > 1) { - GuideStar tmp; - tmp.Find(&image, searchRegion, it->x, it->y, FIND_CENTROID, pFrame->pGuider->GetMinStarHFD(), maxHFD, - pCamera->GetSaturationADU(), FIND_LOGGING_VERBOSE); - // We're repeating the find, so we're vulnerable to hot pixels and creation of unwanted duplicates - if (tmp.WasFound() && tmp.SNR >= minSNR) + for (std::set::reverse_iterator it = stars.rbegin(); it != stars.rend(); ++it) { - bool duplicate = std::find_if(foundStars.begin(), foundStars.end(), [&tmp](const GuideStar& other) - { return CloseToReference(tmp, other); }) != foundStars.end(); - - if (!duplicate) + GuideStar tmp; + tmp.Find(&image, searchRegion, it->x, it->y, FIND_CENTROID, pFrame->pGuider->GetMinStarHFD(), maxHFD, + pCamera->GetSaturationADU(), FIND_LOGGING_VERBOSE); + // We're repeating the find, so we're vulnerable to hot pixels and creation of unwanted duplicates + if (tmp.WasFound() && tmp.SNR >= minSNR) { - tmp.referencePoint.X = tmp.X; - tmp.referencePoint.Y = tmp.Y; - foundStars.push_back(tmp); + bool duplicate = + std::find_if(foundStars.begin(), foundStars.end(), + [&tmp](const GuideStar& other) { return CloseToReference(tmp, other); }) != foundStars.end(); + + if (!duplicate) + { + tmp.referencePoint.X = tmp.X; + tmp.referencePoint.Y = tmp.Y; + foundStars.push_back(tmp); + } } } } @@ -1118,6 +1122,12 @@ bool GuideStar::AutoFind(const usImage& image, int extraEdgeAllowance, int searc Debug.Write("MultiStar: primary star forcibly inserted in list\n"); } } + else + { + tmp.referencePoint.X = tmp.X; + tmp.referencePoint.Y = tmp.Y; + foundStars.push_back(tmp); + } return true; } } From 0017d773dc05cbca0ca233fb2c58b3c74e636a44 Mon Sep 17 00:00:00 2001 From: Bruce Waddington Date: Sat, 9 Nov 2024 09:58:07 -0800 Subject: [PATCH 2/6] Force rebuild of MultiStar list when subframes are disabled --- src/camera.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/camera.cpp b/src/camera.cpp index 6931eaa41..16061c4f5 100644 --- a/src/camera.cpp +++ b/src/camera.cpp @@ -1185,8 +1185,14 @@ void CameraConfigDialogCtrlSet::UnloadValues() if (m_pCamera->HasSubframes) { - m_pCamera->UseSubframes = m_pUseSubframes->GetValue(); - pConfig->Profile.SetBoolean("/camera/UseSubframes", m_pCamera->UseSubframes); + bool oldVal = m_pCamera->UseSubframes; + bool newVal = m_pUseSubframes->GetValue(); + m_pCamera->UseSubframes = newVal; + pConfig->Profile.SetBoolean("/camera/UseSubframes", newVal); + // MultiStar can't track secondary star locations during periods when subframes are used + if (oldVal && !newVal) + if (pFrame->pGuider->GetMultiStarMode()) + pFrame->pGuider->SetMultiStarMode(true); // Will force a refresh of secondary stars } if (m_pCamera->HasGainControl) From 1904c98b12f055c923123ecb5dc809233cf19938 Mon Sep 17 00:00:00 2001 From: Bruce Waddington Date: Sun, 8 Dec 2024 19:27:13 -0800 Subject: [PATCH 3/6] Add try/catch protection in gp regularize_dataset for dithering only --- .../src/gaussian_process_guider.cpp | 85 ++++++++++++------- 1 file changed, 55 insertions(+), 30 deletions(-) diff --git a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp index a1b35f415..96d6cc2ab 100644 --- a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp +++ b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp @@ -274,7 +274,17 @@ double GaussianProcessGuider::result(double input, double SNR, double time_step, { dithering_active_ = false; } - deduceResult(time_step); // just pretend we would do dark guiding... + try + { + deduceResult(time_step); // just pretend we would do dark guiding... + } + catch (std::runtime_error err) + { + reset(); + std::string outStr = "PPEC: Model reset after exception: " + std::string(err.what()); + GPDebug->Log(outStr.c_str()); + return parameters.control_gain_ * input; + } GPDebug->Log("PPEC rslt(dithering): input = %.2f, final = %.2f", input, parameters.control_gain_ * input); @@ -689,38 +699,53 @@ Eigen::MatrixXd GaussianProcessGuider::regularize_dataset(const Eigen::VectorXd& int j = 0; for (size_t i = 0; i < N - 1; ++i) { - if (timestamps(i) < last_cell_end + grid_interval) + try { - gear_error_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_gear_error + gear_error(i)); - variance_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_variance + variances(i)); - last_timestamp = timestamps(i); - } - else - { - while (timestamps(i) >= last_cell_end + grid_interval) + if (timestamps(i) < last_cell_end + grid_interval) { - double inter_timestamp = last_cell_end + grid_interval; - - double proportion = (inter_timestamp - last_timestamp) / (timestamps(i) - last_timestamp); - double inter_gear_error = proportion * gear_error(i) + (1 - proportion) * last_gear_error; - double inter_variance = proportion * variances(i) + (1 - proportion) * last_variance; - - gear_error_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_gear_error + inter_gear_error); - variance_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_variance + inter_variance); - - reg_timestamps(j) = last_cell_end + 0.5 * grid_interval; - reg_gear_error(j) = gear_error_sum / grid_interval; - reg_variances(j) = variance_sum / grid_interval; - - last_timestamp = inter_timestamp; - last_gear_error = inter_gear_error; - last_variance = inter_variance; - last_cell_end = inter_timestamp; - - gear_error_sum = 0.0; - variance_sum = 0.0; - ++j; + gear_error_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_gear_error + gear_error(i)); + variance_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_variance + variances(i)); + last_timestamp = timestamps(i); } + else + { + while (timestamps(i) >= last_cell_end + grid_interval) + { + if (dithering_active_) // generalizing this will require recovery in any function that calls UpdateGP + { + if (j >= reg_timestamps.size()) + { + GPDebug->Log("PPDbg: Index-over-run in regularize_dataset, j = %d", j); + throw std::runtime_error("Index over-run in regularize_dataset"); + } + } + double inter_timestamp = last_cell_end + grid_interval; + + double proportion = (inter_timestamp - last_timestamp) / (timestamps(i) - last_timestamp); + double inter_gear_error = proportion * gear_error(i) + (1 - proportion) * last_gear_error; + double inter_variance = proportion * variances(i) + (1 - proportion) * last_variance; + + gear_error_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_gear_error + inter_gear_error); + variance_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_variance + inter_variance); + + reg_timestamps(j) = last_cell_end + 0.5 * grid_interval; + reg_gear_error(j) = gear_error_sum / grid_interval; + reg_variances(j) = variance_sum / grid_interval; + + last_timestamp = inter_timestamp; + last_gear_error = inter_gear_error; + last_variance = inter_variance; + last_cell_end = inter_timestamp; + + gear_error_sum = 0.0; + variance_sum = 0.0; + ++j; + } + } + } + catch (std::runtime_error err) + { + throw err; } } if (j > REGULAR_BUFFER_SIZE) From e7aef5d675bd98bd23effd68a9e687c3ed6193bb Mon Sep 17 00:00:00 2001 From: Bruce Waddington Date: Sun, 15 Dec 2024 09:40:51 -0800 Subject: [PATCH 4/6] PR changes. Restore star.cpp to resolve Clang issues --- .../src/gaussian_process_guider.cpp | 67 +++++++++---------- src/star.cpp | 5 +- 2 files changed, 33 insertions(+), 39 deletions(-) diff --git a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp index 96d6cc2ab..d94c31bf1 100644 --- a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp +++ b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp @@ -680,6 +680,7 @@ void GaussianProcessGuider::UpdatePeriodLength(double period_length) SetGPHyperparameters(hypers); // the setter function is needed to convert parameters } +// NOTE: Callers must be prepared to handle a thrown exception Eigen::MatrixXd GaussianProcessGuider::regularize_dataset(const Eigen::VectorXd& timestamps, const Eigen::VectorXd& gear_error, const Eigen::VectorXd& variances) { @@ -699,54 +700,48 @@ Eigen::MatrixXd GaussianProcessGuider::regularize_dataset(const Eigen::VectorXd& int j = 0; for (size_t i = 0; i < N - 1; ++i) { - try + + if (timestamps(i) < last_cell_end + grid_interval) { - if (timestamps(i) < last_cell_end + grid_interval) - { - gear_error_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_gear_error + gear_error(i)); - variance_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_variance + variances(i)); - last_timestamp = timestamps(i); - } - else + gear_error_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_gear_error + gear_error(i)); + variance_sum += (timestamps(i) - last_timestamp) * 0.5 * (last_variance + variances(i)); + last_timestamp = timestamps(i); + } + else + { + while (timestamps(i) >= last_cell_end + grid_interval) { - while (timestamps(i) >= last_cell_end + grid_interval) + if (dithering_active_) // generalizing this will require recovery in any function that calls UpdateGP { - if (dithering_active_) // generalizing this will require recovery in any function that calls UpdateGP + if (j >= reg_timestamps.size()) { - if (j >= reg_timestamps.size()) - { - GPDebug->Log("PPDbg: Index-over-run in regularize_dataset, j = %d", j); - throw std::runtime_error("Index over-run in regularize_dataset"); - } + GPDebug->Log("PPDbg: Index-over-run in regularize_dataset, j = %d", j); + throw std::runtime_error("Index over-run in regularize_dataset"); } - double inter_timestamp = last_cell_end + grid_interval; + } + double inter_timestamp = last_cell_end + grid_interval; - double proportion = (inter_timestamp - last_timestamp) / (timestamps(i) - last_timestamp); - double inter_gear_error = proportion * gear_error(i) + (1 - proportion) * last_gear_error; - double inter_variance = proportion * variances(i) + (1 - proportion) * last_variance; + double proportion = (inter_timestamp - last_timestamp) / (timestamps(i) - last_timestamp); + double inter_gear_error = proportion * gear_error(i) + (1 - proportion) * last_gear_error; + double inter_variance = proportion * variances(i) + (1 - proportion) * last_variance; - gear_error_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_gear_error + inter_gear_error); - variance_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_variance + inter_variance); + gear_error_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_gear_error + inter_gear_error); + variance_sum += (inter_timestamp - last_timestamp) * 0.5 * (last_variance + inter_variance); - reg_timestamps(j) = last_cell_end + 0.5 * grid_interval; - reg_gear_error(j) = gear_error_sum / grid_interval; - reg_variances(j) = variance_sum / grid_interval; + reg_timestamps(j) = last_cell_end + 0.5 * grid_interval; + reg_gear_error(j) = gear_error_sum / grid_interval; + reg_variances(j) = variance_sum / grid_interval; - last_timestamp = inter_timestamp; - last_gear_error = inter_gear_error; - last_variance = inter_variance; - last_cell_end = inter_timestamp; + last_timestamp = inter_timestamp; + last_gear_error = inter_gear_error; + last_variance = inter_variance; + last_cell_end = inter_timestamp; - gear_error_sum = 0.0; - variance_sum = 0.0; - ++j; - } + gear_error_sum = 0.0; + variance_sum = 0.0; + ++j; } } - catch (std::runtime_error err) - { - throw err; - } } if (j > REGULAR_BUFFER_SIZE) { diff --git a/src/star.cpp b/src/star.cpp index 472f1d967..d7aa5bec5 100644 --- a/src/star.cpp +++ b/src/star.cpp @@ -1031,9 +1031,8 @@ bool GuideStar::AutoFind(const usImage& image, int extraEdgeAllowance, int searc // We're repeating the find, so we're vulnerable to hot pixels and creation of unwanted duplicates if (tmp.WasFound() && tmp.SNR >= minSNR) { - bool duplicate = - std::find_if(foundStars.begin(), foundStars.end(), - [&tmp](const GuideStar& other) { return CloseToReference(tmp, other); }) != foundStars.end(); + bool duplicate = std::find_if(foundStars.begin(), foundStars.end(), [&tmp](const GuideStar& other) + { return CloseToReference(tmp, other); }) != foundStars.end(); if (!duplicate) { From feadcc285f6955e98651b13f6190588b670b509e Mon Sep 17 00:00:00 2001 From: Bruce Waddington Date: Sun, 15 Dec 2024 10:07:57 -0800 Subject: [PATCH 5/6] PR-requested changes, logging, 'const' --- .../src/gaussian_process_guider.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp index 8cf6effa9..f46642192 100644 --- a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp +++ b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp @@ -278,11 +278,13 @@ double GaussianProcessGuider::result(double input, double SNR, double time_step, { deduceResult(time_step); // just pretend we would do dark guiding... } - catch (std::runtime_error err) + catch (const std::runtime_error err) { reset(); - std::string outStr = "PPEC: Model reset after exception: " + std::string(err.what()); - GPDebug->Log(outStr.c_str()); + std::ostringstream message; + message << "PPEC: Model reset after exception: " << err.what(); + GPDebug->Write(message.str().c_str()); + return parameters.control_gain_ * input; } From e03416e51b6a9cae2498a9802d18fd50d1363a94 Mon Sep 17 00:00:00 2001 From: Bruce Waddington Date: Sun, 15 Dec 2024 12:28:20 -0800 Subject: [PATCH 6/6] PR request, exception pass by reference --- .../MPI_IS_gaussian_process/src/gaussian_process_guider.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp index f46642192..8b6c62986 100644 --- a/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp +++ b/contributions/MPI_IS_gaussian_process/src/gaussian_process_guider.cpp @@ -278,7 +278,7 @@ double GaussianProcessGuider::result(double input, double SNR, double time_step, { deduceResult(time_step); // just pretend we would do dark guiding... } - catch (const std::runtime_error err) + catch (const std::runtime_error& err) { reset(); std::ostringstream message;