C5 W3 Trigger word detection - minor error in Unit Test for insert_ones

Hello,

I just wanted to report a minor error I have spotted in one of the Unit Tests, which have little chance to show up, but I hope to help by flagging it anyways.
I’m referring to the function ‘insert_ones_test’ and the line ‘assert arr1[0][segment_end_y + 51] == 0’. As the upper bound of the random range for segment_end_y is set to 1324, when the random selector picks 1324, the above line will currently throw an error, due to the list having only 1375 values.
An if statement checking if segment_end_y < 1324 should do the job.

Many thanks,
Antonio

Thanks for your report, I’ll submit a support ticket.

Ok, I finally (after 4 years) got to that assignment just now. I would say that the problem with the unit tests there is way more extensive than that. They specifically say in the definition of the function that you have to handle the case that the segment end is less than 50 units from the end of the interval by being careful not to index off the end. But then the tests are very specifically making sure they don’t test for that behavior.

If they enhanced them to handle that case, then the more specific problem (a classic “off by one” error) that you point out would be handled as a side effect. My claim is that the tests should be expanded to cover the “running off the end” scenario, since that is part of the function definition.

Tom, since I’m “piling on” here with more requirements, I’m happy to file the enhancement request.

Actually here’s another thing I haven’t checked yet: how the grader handles this. You can pass the unit tests without handling the “off the end” issue, but does code that doesn’t handle it get full score from the grader? Stay tuned for news at 11!

Ok, there’s more sketchy stuff here. The way they have us write insert_audio_clip, it actually modifies the global variable that is passed in as the previous_segments argument. I added the logic to do the append after the call to insert_audio_clip from create_training_example, but I ended up with twice as many entries as expected. The grader did not care, though.

So maybe I’m just being a little too puritanical here, but this feels kind of unclean. At the very least, they should call it out. The docstring says there is only one return value from insert_audio_clip. That’s true in a literal sense, but the function has a side effect.

Using global variables, and modifying them without documentation, are both poor practices.

I think your assessment likely explains some history of unexplained behavior.

It would be great if you proceed with filing a ticket for this set of issues. I have not started one yet (I was saving it as a holiday treat :grinning:).

Sure, I’ll formulate the github request. I’m making my list and checking it twice! :laughing: Haven’t gotten to testing how insert_ones grading reacts to not handling “off the end” issues.

I must say that the W3 assignments have reached an extremely elevated level of hand-holding. This assignment in particular felt more like transcription than programming: they did everything but actually type out the final code for you. I’m especially curious to see what happens when I finally get to Week 4, because there have been lots of complaints about that material not doing a sufficient level of hand-holding. I guess we are showing our true colors here: working Week 4 was going to be my Holiday Treat! :nerd_face: :santa: Need a “nerd Santa” emoji!

From the student level, Week 4 is very heavy snow.

Sorry to be slow getting the github request filed, but I did finally confirm that as expected, the grader will pass your insert_ones function even if you write it to specifically not do anything in the case that you hit the end. Meaning literally not add any ones at all in that case. They do have a visual example showing the off the end behavior in the notebook, so you don’t get a picture like the “Expected Value” as shown, but your code will pass everything.