Skip to content

Add Pass Feature Collection class#3599

Open
sauravbanna wants to merge 23 commits intomasterfrom
sauravbanna/test_pass_data_collection
Open

Add Pass Feature Collection class#3599
sauravbanna wants to merge 23 commits intomasterfrom
sauravbanna/test_pass_data_collection

Conversation

@sauravbanna
Copy link
Copy Markdown
Contributor

@sauravbanna sauravbanna commented Feb 12, 2026

Description

Added a class which logs Pass Features -> features we'd like to use for training weights for judging passes in the future

I put the new class in the passing directory since it made sense

Testing Done

Compiles + logs at the correct rate based on boolean flag. next pr will consume the protos generated

Resolved Issues

Length Justification and Key Files to Review

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

  • Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • Remove all commented out code
  • Remove extra print statements: for example, those just used for testing
  • Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

@sauravbanna sauravbanna marked this pull request as ready for review February 18, 2026 09:14
Copy link
Copy Markdown
Contributor

@Andrewyx Andrewyx left a comment

Choose a reason for hiding this comment

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

left some small nits, looks good so far!

Comment on lines +7 to +9
//
// Created by thunderbots on 2/9/26.
//
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: stray comment

double min_interception_distance =
std::max(0.0, enemy_interception_vector.length() - ROBOT_MAX_RADIUS_METERS);

const double ENEMY_ROBOT_INTERCEPTION_SPEED_METERS_PER_SECOND = 0.5;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How was this speed determined & if the intention was that this value should be tuned, we should add a TODO here to track. Otherwise, we should move this constant elsewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it was used in the old cost function code, i can move it to a constants file

min_interception_distance, ENEMY_ROBOT_MAX_SPEED_METERS_PER_SECOND,
ENEMY_ROBOT_MAX_ACCELERATION_METERS_PER_SECOND_SQUARED,
signed_1d_enemy_vel, ENEMY_ROBOT_INTERCEPTION_SPEED_METERS_PER_SECOND)
.toSeconds();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caller should perform conversion. As a wrapper around getTimeToTravelDistance, this function should share a similar return signature type.

0.0, 1.0);
}

const Robot* getClosestReceiverToPass(const Team& friendly_team, const Pass& pass)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any reason we are using raw pointers here? e.g.const Robot*

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

double ratePassFriendlyCapability(const Team& friendly_team, const Pass& pass,
const TbotsProto::PassingConfig& passing_config);

Rectangle getReducedField(const Field& field, TbotsProto::PassingConfig passing_config);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comments

score += BAD_SCORE;

// how well the receiver can receive the ball
auto friendlyReceiveCapabilitySigmoid =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

its a double

// how well any enemy can intercept the ball
for (const auto& robot : world.enemyTeam().getAllRobots())
{
auto intercept_time_delta =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

double

}

num_passes_since_sample_ =
(num_passes_since_sample_ + 1) % PASS_SAMPLING_FREQUENCY;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

num_passes_since_sample_++

{
if (num_picks_since_random_ == 0)
{
std::uniform_int_distribution<std::size_t> distribution(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see if slow

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the main overhead seems to be the std::mt19937 random_num_gen_; initialization, which is only done once. the uniform_int_distribution has to be initialized every time since the size of the considered passes list changes, but doesn't seem to be a huge overhead.

Comment on lines +87 to +89
unsigned int num_passes_since_sample_ = 0;
unsigned int num_picks_since_random_ = 0;
bool sample_pass_features_ = false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

constructor

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