-
Notifications
You must be signed in to change notification settings - Fork 24
Create an updated version of autoencoder tutorial #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create an updated version of autoencoder tutorial #194
Conversation
… added some more plots. Metrics still a work in progress.
|
@tennlee I've created a draft PR, so that you can see the general direction I'm headed in and whether that direction is accetable to as PET acting BDFL... |
Pull Request Test Coverage Report for Build 19197223590Details
💛 - Coveralls |
…opefully added some functionality that will provide some suggestions on how participants in the hackathon might try this as an exercise and measure improvement.
|
This is looking good. I think the cell where the tutorial displays the prediction from the untrained network could do with an explanation. People have always been interested to see what a prediction from an untrained network looks like, because it's not totally unstructured. I also think it's good to see the progression from 'untrained' to 'trained' playing itself out. |
|
For the example near the end showing the input, prediction and error - this is very helpful to see - maybe consider a bigger gap between samples so they look a bit more different from one another. |
|
Having the score calculations at the end is good. It might be worth explaining SSIM is not a perfect metric even though it can be useful. I really like these additions. I am going to go ahead and merge the work so that it's there for Monday morning, but if you can circle back to the comments and address them at some point, I think it would be worthwhile. |
|
Hi Stephen - I think your PR contains some files you didn't mean to change. Can you review the changed files and remove any files which were not intended to be changed? Also please run 'black' on time.py? Also, the original AutoEncoder tutorial has been slightly updated on develop to correct the display of the final image - please update your version also accordingly. Or, if you are putting all the changes into the new notebook, just remove AutoEncoder original from your PR. |
|
time.py and the new notebook look good. I might use some git tricks to get those onto develop, I am not sure what that will do to this PR however, so when I'm done the diff for this PR might look a little strange! |
|
Okay, so apparently what I did had the side effect of closing this PR, which was unintended. I thought it would leave it open, with some files still not merged. Please also bring test coverage of time.py back up to 100% |
Here is my update to the autoenccoder tutorial.
(more stuff on the way)