Skip to content
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

fix: disable_line_search (#21) #78

Open
wants to merge 2 commits into
base: x1eve/beta/v0.1.0
Choose a base branch
from

Conversation

kimurariku
Copy link

@kimurariku kimurariku commented Jan 9, 2025

PR Type

  • Improvement

Related Links

Internal Link

Description

The purpose of this PR is to incorporate off line search to improve NDT convergence.

Review Procedure

The commit are cerry-picked from tier4/main

It already discussed in this PR.

The commit 6cf5fe6 of this PR is converted to 9a0877a fwhen it was merged.

Remarks

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Code follows [coding guidelines][coding-guidelines]
  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • Code follows [coding guidelines][coding-guidelines]
  • (Optional) Unit tests have been written for new behavior
  • PR title describes the changes
  • (When added something to .repos) Check if proper access rights are set.
    You can check the latest permission setting status of each repository [here][github-repository-status].
    If the repository permissions are insufficient, you need to add the permissions according to [this][repository-permission-setting-flow].

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets
  • Write [release notes][release-notes]

CI Checks

  • vcs import: Required to pass before the merge.
  • Check spelling: NOT required to pass before the merge. It is up to the reviewer(s). See here if you want to add some words to the spell check dictionary.

@sfukuta
Copy link

sfukuta commented Jan 10, 2025

Since the "include/multigrid_pclomp" folder does not exist, there is no need to add the "hpp" file.

@sfukuta
Copy link

sfukuta commented Jan 10, 2025

Are the changes consistent with PR #21 ?

@kimurariku
Copy link
Author

@sfukuta

Since the "include/multigrid_pclomp" folder does not exist, there is no need to add the "hpp" file.

The change in this file is to comment out unnececary file, so nothing to need adding in this PR.
I drop this change.

@kimurariku
Copy link
Author

Are the changes consistent with PR #21 ?

include/multigrid_pclomp/multigrid_ndt_omp_impl.hpp not needed and should remain consistent.

@kimurariku
Copy link
Author

include/multigrid_pclomp/multigrid_ndt_omp_impl.hpp is deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants