Closed DonaldAlan closed 1 year ago
@DonaldAlan this looks good. Do you mind submitting an ECA? https://deeplearning4j.konduit.ai/multi-project/how-to-guides/contribute/eclipse-contributors#signing-the-eclipse-contributor-agreement
I created an Eclipse account and signed the legal agreement. Thanks.
Do I need to sign all the commits?
@DonaldAlan just your most recent one I believe. Main thing is having your git email in the eclipse system. Thanks!
I can't push. I tried git commit --amend --signoff
; that didn't work, so I did a new commit with commit -s ...
tried to push with git push https://github.com/DonaldAlan/dl4j-examples
. That caused an error with
error: failed to push some refs to 'https://github.com/DonaldAlan/dl4j-examples'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
But neither git pull
nor git pull --rebase
does anything. I don't believe anyone else pushed to my branch. So I'm kinda lost about how to proceed. I'm googling stuff about git to figure out what to do, but if you know what to do, please tell me. Thanks.
Ok, I got it to work by doing git pull https://github.com/DonaldAlan/dl4j-examples meldoy4j-review-971
and then git push https://github.com/DonaldAlan/dl4j-examples meldoy4j-review-971
. I'd expect you may have to re-approve because I pushed a small change (showing time elapsed per epoch). Thanks.
What changes were proposed in this pull request?
I cleaned up the code for melody learning from MIDI.
I fixed a bug related to pitches that were out of range (the old code adjusted them by more than an octave, which would make the music sound bad).
I added documentation, including about how to switch to a different MIDI zip file from the default (bach-midi.zip).
I made the code more modifiable (e.g., if you want to increase the range of allowed pitch deltas).
I moved the melody4j package from the wip/ folder to org.deeplearning4j.examples.advanced.modelling.charmodelling.melodl4j.
I made no other changes to dl4j-examples; I pulled from master right before my changes.
How was this patch tested?
I ran it on a clean temporary directory (deleting old zip files, midi files, and melody string files) and verified that it learns reasonable-sounding melodies and that it plays them after epochs. Also, I added the class TestMelodyConversion.java that checks that converting from MIDI to strings and then back to MIDI results in MIDI that sounds approximately the same. Too, it runs a junit-like method that verifies that conversion of pitches to chars and then of those chars back to pitches preserves the pitch.
Please review https://github.com/eclipse/deeplearning4j/blob/master/CONTRIBUTING.md before opening a pull request.