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

Evaluate accuracy #26

Merged
merged 35 commits into from
Apr 14, 2020
Merged

Conversation

cbachhuber
Copy link
Contributor

Addressing #23: First version of accuracy evaluation. The evaluator assigns clusters to the closest ground truth vehicles. For the overall summary, it does not yet distinguish vehicles. For simplicity, it lumps the errors from all vehicles into shared cumulative variables.

@cbachhuber cbachhuber mentioned this pull request Apr 13, 2020
…dynamic-occupancy-grid-map into cbachhuber-evaluate-performance

# Conflicts:
#	dogm/demo/precision_evaluator.h
@TheCodez
Copy link
Owner

Awesome work as always 👍
A few changes and this is ready to go.

Copy link
Owner

@TheCodez TheCodez left a comment

Choose a reason for hiding this comment

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

Can you merge my pull request to make the dbscan api more appealing
cbachhuber#1

@cbachhuber
Copy link
Contributor Author

Good points, thanks for those improvements! Merged.

@TheCodez
Copy link
Owner

You can add the std::optional changes to this pr as well if you want.

@cbachhuber
Copy link
Contributor Author

Thanks, done :)

@TheCodez
Copy link
Owner

TheCodez commented Apr 13, 2020

I checked this out and sadly Visual Studio has no <experimental/optional> for C++ 14. Optional is only available starting at version 17.

@cbachhuber
Copy link
Contributor Author

cbachhuber commented Apr 14, 2020

... and I expected that for once, we would not have issues with MSVC 😅

Which version of Visual studio are you using? Could it be an older one that does not yet support std::experimental for C++14?

If we cannot solve this through Visual studio, I see a couple of options here:

  • Use boost's optional. Optional was first proposed in boost. However, this would introduce an additional dependency for this project
  • Implement our own optional (it's a really simple class)/copypaste from e.g. here
  • Import the above repo as git submodule
  • Do not use optional (least favorable for imho)

All this aside: in general, I suggest removing optional from this PR. Let's deal with it separately, this PR is already huge 😄

@TheCodez
Copy link
Owner

TheCodez commented Apr 14, 2020

I would remove optional from this pr.
Given there’s need to use optional every in the codebase and the small use case here, I see no reason to add another dependency.

This could change of course at some point.

@TheCodez
Copy link
Owner

I tested with Visual Studio 2019 and the error persists. If there are more cases where optional could be used I would go with option 2 or 3.

@cbachhuber
Copy link
Contributor Author

Thanks for checking. I removed optional, should be ready to merge now.

If I find time, I'll introduce optional to this repo. But first, I think I want to tidy up in folder dogm/demo ;)

@TheCodez TheCodez merged commit ee13ee9 into TheCodez:master Apr 14, 2020
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.

2 participants