udacity / sdc-issue-reports

29 stars 6 forks source link

Term 2, Lesson 11, Solution: Initialize Priors Function bad/wrong code #1426

Closed DominikMa closed 5 years ago

DominikMa commented 5 years ago

In Term 2 Lesson 11 the solution for the quiz Initialize Priors Function contains code which just works for the given example.

Changing the landmarks line from std::vector<float> landmark_positions {5, 10, 20}; to std::vector<float> landmark_positions {0, 10, 20}; for example throws an error because the prior vector is accessed at -1.

Even worse setting two landmarks next to each other std::vector<float> landmark_positions {9, 10, 20}; results in a wrong output. The probabilities should add up and not be overwritten.

I know it is just a quit but in my opinion students should not be encouraged to write wrong code.

A simple solution (assuming the map is cyclic) could be

float normalization_term = landmark_positions.size() * (position_stdev * 2 + 1);
for (auto& landmark_position : landmark_positions){
  for(float i=1; i<=position_stdev; i++){
    priors.at((int)(i+landmark_position+map_size)%map_size) += 1.0/normalization_term;
    priors.at((int)(-i+landmark_position+map_size)%map_size) += 1.0/normalization_term;
  }
  priors.at(landmark_position) += 1.0/normalization_term;
}
mvirgo commented 5 years ago

Good catch here - I've updated this to line up with your proposed solution. Thanks for noting this!