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

Improvements for ellipse fitting #66

Merged
merged 9 commits into from
Jun 20, 2018
Merged

Improvements for ellipse fitting #66

merged 9 commits into from
Jun 20, 2018

Conversation

simogasp
Copy link
Member

It introduces some checks that avoid fitting an ellipse if there are less than 5 points.

Also it introduces unit testing with boost Test

connected to #65

* added doc
* init() -> private and used inside setParameters()
* inclass member initialization with default ctor
@simogasp simogasp added this to the V1.0.0 milestone Feb 27, 2018
@fabiencastan
Copy link
Member

@lcalvet @zvrba @griwodz Can you review before merge?

@simogasp
Copy link
Member Author

Actually I'm working with @lcalvet on the dataset evaluation so that we can assure that none of the changes here has affected the quality of results.
A little bit more of time but it should be something that we can use later to assess the PR in terms of results

@lcalvet
Copy link
Member

lcalvet commented Jun 20, 2018

Full comparisons of this branch against develop have been performed, namely:

  • imaged center accuracy (synthetic images),
  • detection rate (synthetic images),
  • confusion rate (synthetic images),
  • false positive (real images).
    The branch can be merged. Thanks @simogasp !

@lcalvet lcalvet merged commit 568c489 into develop Jun 20, 2018
@simogasp simogasp deleted the specialCases branch June 20, 2018 21:25
@simogasp
Copy link
Member Author

report.pdf
Here is a stub of the report of the tests (to be improve graphically but with all the needed figures)

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

Successfully merging this pull request may close these issues.

3 participants