Skip to content
This repository was archived by the owner on Mar 9, 2026. It is now read-only.

fix the bug for threshold update - #148

Merged
lpanaf merged 5 commits into
mainfrom
lpanaf/fix_threshold_setting
Dec 13, 2024
Merged

lpanaf merged 5 commits into
mainfrom
lpanaf/fix_threshold_setting

Conversation

@lpanaf

@lpanaf lpanaf commented Dec 12, 2024

Copy link
Copy Markdown
Collaborator

fix the bug that the update of threshold from command line is not reflected in the structure

@lpanaf
lpanaf requested a review from ahojnnes December 12, 2024 14:31
Comment thread glomap/estimators/bundle_adjustment.h Outdated
// Constrain the minimum number of views per track
int min_num_view_per_track = 3;

void UpdateThreshold() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of this fix, I suggest the following:

struct BundleAdjusterOptions {
  double thres_loss_function = 1;

  std::shared_ptr<ceres::LossFunction> CreateLossFunction() {
    return std::make_shared<ceres::HuberLoss>(thres_loss_function);
  }
};

Since you explicitly have to call CreateLossFunction(), it is harder to accidentally forget calling the UpdateThreshold().

@ahojnnes

Copy link
Copy Markdown
Contributor

The proposal was to return the shared pointer and not assign it. In the respective BA and GO you would then call this method to create the loss function (always with the latest threshold).

@lpanaf
lpanaf merged commit ebd50af into main Dec 13, 2024
uptycs-rmack pushed a commit to uptycs-rmack/glomap that referenced this pull request Jan 1, 2026
* fix the bug that the update of threshold from command line is not reflected in the structure

* f

* update

* refactor the loss_function

* d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants