-
Notifications
You must be signed in to change notification settings - Fork 149
Conversation
Any comments on this model ? @saeta @dan-zheng |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few minor formatting changes, but in addition to those could you add this as a buildable target for CMake?
Aside from that, the model looks like it matches the reference implementation nicely, so we'll be glad to pull it in after those few minor tweaks. Thanks for working on this.
Hi @BradLarson , sorry about the late reply. Missed this notification!! Yeah I'll work on the CMake target and request a review again!! I'll also be adding the 2D implementation soon. |
Just checking in again, we'd love to be able to pull this in with the few small suggestions I've made here. |
HI @BradLarson sorry for the delay. Been busy with assignments lately. You should be expecting the suggested changes in a day or two!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope you don't mind, but I've gone ahead and pushed fixes for the several remaining issues. I reformatted with swift-format, fixed the CMakeLists in various places for the autoencoder targets, and updated the datasets for use with the new (soon to be migrated) Batcher syntax. This builds, but correct functioning may still need to be verified.
I'm pulling this in as it is, and if you wish to further improve this, we can follow on with a new PR.
Added a probabilistic programming based 1D VariationalAutoencoder based on PyTorch's implementation and previous Autoencoder1D implementation.