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

Add NDT benchmark report. Adjust default parameter values for the NDT sensor model. #395

Closed
wants to merge 3 commits into from

Conversation

serraramiro1
Copy link
Contributor

@serraramiro1 serraramiro1 commented May 31, 2024

Proposed changes

Adds a new benchmark report for the NDT sensor model for magazino datasets.
Matches NDT sensor model default parameters in the example launch files to match the ones that got empirically tested.
Related to #316.

Type of change

  • 🐛 Bugfix (change which fixes an issue)
  • 🚀 Feature (change which adds functionality)
  • 📚 Documentation (change which fixes or extends documentation)

Checklist

Put an x in the boxes that apply. This is simply a reminder of what we will require before merging your code.

  • Lint and unit tests (if any) pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All commits have been signed for DCO

@serraramiro1
Copy link
Contributor Author

@hidmic I'd love your thoughts on how to best present this benchmark given that it doesn't follow the current one's format (we can't re-generate it from this repo alone).

@serraramiro1 serraramiro1 added documentation Improvements or additions to documentation enhancement New feature or request labels May 31, 2024
@hidmic
Copy link
Collaborator

hidmic commented May 31, 2024

We were talking precisely about this with @glpuga and @nahueespinosa earlier today. A report pulled out of nowhere is not ideal. Neither having old reports in our repository that are no longer representative of the current system. We need the benchmarking code out, that's on me. Until then, we should probably keep this elsewhere. Thoughts?

@nahueespinosa
Copy link
Member

Yeah, adding a pdf to this repo might not be the best. I think it makes more sense to move in the opposite direction until we can open source our benchmarking code. We were talking about removing the beluga_benchmark package even.

I'd keep the default parameter changes though.

@serraramiro1
Copy link
Contributor Author

Will create a new one so we keep this closed PR as-is for post-facto analysis.

@nahueespinosa nahueespinosa deleted the ramiro/add-ndt-benchmark-report branch June 15, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants