Dead code?

[martingasser]

Hi,

first of all, congratulations to this work!

During examining the model, I noticed that x_contours is not used and overwritten after https://github.com/spotify/basic-pitch/blob/fc5c3191281149c54e0b788738ac7ddecbda81a8/basic_pitch/models.py#L207-L216

I also wanted to ask if you plan on releasing the script used for training the model as well.

Thanks, Martin

[drubinstein]

Hi @martingasser !

Good find. Feel free to PR a change or I will when I have time to update the code.

Regarding releasing the code for training, we intend to push it sometime during the summer. Thanks for your patience.

[eunkoh]

Hi @drubinstein I can see your branch with train.py but it's hard to use this branch without any example or explanation. Could you share any examples for the branch? Basically, I am interested in using my own data for training, but it's hard to reproduce the training with your data as well. Any information would be greatly appreciated. Many thanks.

https://github.com/spotify/basic-pitch/blob/drubinstein/wip-training/basic_pitch/train.py

Hi @martingasser !

Good find. Feel free to PR a change or I will when I have time to update the code.

Regarding releasing the code for training, we intend to push it sometime during the summer. Thanks for your patience.

[eunkoh]

@drubinstein For example, do I need *tfrecord for train/valid split for training? line 232 in https://github.com/spotify/basic-pitch/blob/drubinstein/wip-training/basic_pitch/dataset/tf_example_deserialization.py

[bgenchel]

Hey @eunkoh, I'm planning on working on the training code in the near future, so i'll try to answer your questions to the best of my ability. I think it's worth pointing out that the code in that branch is still 'wip' as labeled, so we're aware on our side that it doesn't work in it's current form.

As for your question about *tfrecord, I'm guessing that you're referring to the specific argument in os.join. This just specifies the file extension that should be read in the preceding folder in the path (the split var) . The split var looks to contain a string, either "valid" or "train", which will determine what generator function will be used in the subsequent lines.

Please let me know if i'm misunderstanding your question and clarify further if so!

[TeeJayBaker]

Hey @drubinstein @rabitt, Sorry to rehash this after so long, just a question as to the model structure, as I'm trying to transfer the weights dict to port to a PyTorch model.

Was in intended to be the reduced version, with the three skipped layers that are still present in the code? Or was it just a happy accident that it happened to work very well without them? As the model structure diagram in the paper features the original Conv-BatchNorm-ReLU that's been skipped.

If so would you consider retraining with the original intended model structure and see if the model accuracy improves? Any clarification would be much appreciated!