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

More dense candidates #1399

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

More dense candidates #1399

wants to merge 10 commits into from

Conversation

h3ndrk
Copy link
Member

@h3ndrk h3ndrk commented Jul 20, 2024

Why? What?

  • More dense candidate circles s.t. we have better fitting candidates for the neural networks
  • There must be at least 3 segments on a circle to become a candidate
  • Fix field border drawing to be above the field border seeds

ToDo / Known Issues

None

Ideas for Next Iterations (Not This PR)

  • Fix and reenable test cases of perspective grid candidate generator

How to Test

Use the ball detection image overlay or/and ball candidates panel in Twix to see the changes. We should detect the ball better or similar as before.

@h3ndrk h3ndrk enabled auto-merge July 20, 2024 20:10
@h3ndrk h3ndrk disabled auto-merge July 20, 2024 20:17
@h3ndrk h3ndrk force-pushed the more-dense-candidates branch 2 times, most recently from 9fc0887 to 0fa8dca Compare July 21, 2024 07:22
@h3ndrk h3ndrk force-pushed the more-dense-candidates branch from 219bf8e to aed4b61 Compare July 21, 2024 08:23
@h3ndrk h3ndrk enabled auto-merge July 21, 2024 11:07
@schmidma
Copy link
Member

I guess, this is still in progress @h3ndrk?

@h3ndrk
Copy link
Member Author

h3ndrk commented Jul 31, 2024

No, this is ready for review, it is on the board, description is up-to-date.

}
);
}
// TODO: Fix and reenable tests
Copy link
Member

Choose a reason for hiding this comment

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

But this PR still contains TODOs ?!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, as mentioned in the PR description, this can be tackled in a next iteration. These tests don't have much impact.

@oleflb
Copy link
Contributor

oleflb commented Aug 7, 2024

Someone should fix these tests :D

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

Successfully merging this pull request may close these issues.

3 participants