basic-pitch Removed dead code.

This one should do the right thing :-)

Jun 22 '22 15:06 martingasser

Great catch @martingasser ! Actually this is a bug in our code... this line should actually be x_contours, not x. However, the saved model in the repo was trained with the bug, where the code block is indeed "dead code".

@drubinstein thoughts on how we want to handle this?

Jun 22 '22 18:06 rabitt

I think we should leave it as is and add a comment to the README about which commit should be used to match the paper i.e. the one with the bug.

Now that we have the disclaimer that the code will evolve, we can fix the corresponding bug. However, I'm thinking we can hold off on fixing this bug until we add the training, dataset creation and evaluation code if possible.

Another option is we start creating separate basic pitch model repos, one for the ICASSP version which will see no bug updates (ever!) and one with our continuing updates. It may be a bit messy to maintain though. We should move this into an issue discussion instead of keeping it in the PR.

Jun 22 '22 18:06 drubinstein

Thanks for clarifying @rabitt @drubinstein

Looking forward to the dataset creation and training code!

Jun 23 '22 09:06 martingasser

Going to close this one for now - but revisit once we have integrate the training code.

Aug 29 '22 16:08 rabitt