Skip to content
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

Symplectic Autoencoder and PSD architecture #138

Merged
merged 21 commits into from
May 2, 2024

Conversation

benedict-96
Copy link
Collaborator

Implemented a symplectic autoencoder and a PSD architecture. For the PSD architecture we don't perform training but use solve!. They are both subtypes of AutoEncoder. Much of the functionnality for SymplecticAutoencoder and PSDArch is performed by this generic AutoEncoder type.

@benedict-96 benedict-96 requested a review from michakraus April 24, 2024 16:18
Copy link

codecov bot commented Apr 24, 2024

Codecov Report

Attention: Patch coverage is 82.75862% with 25 lines in your changes are missing coverage. Please review.

Project coverage is 68.13%. Comparing base (45908cc) to head (08a4345).
Report is 4 commits behind head on main.

Files Patch % Lines
src/architectures/symplectic_autoencoder.jl 79.16% 10 Missing ⚠️
src/data_loader/batch.jl 47.05% 9 Missing ⚠️
src/architectures/autoencoder.jl 93.33% 2 Missing ⚠️
src/architectures/psd.jl 92.59% 2 Missing ⚠️
src/reduced_system/reduced_system.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #138      +/-   ##
==========================================
+ Coverage   67.37%   68.13%   +0.76%     
==========================================
  Files         116      119       +3     
  Lines        3169     3314     +145     
==========================================
+ Hits         2135     2258     +123     
- Misses       1034     1056      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@michakraus
Copy link
Member

Please rename get_encoder, get_decoder and similar functions and remove the get_.

@benedict-96
Copy link
Collaborator Author

I thought that the get_ may be appropriate in this particular case because we don't actually compute anything or allocate new memory. The SymplecticAutoencoder stores the parameters for the encoder and the decoder (they are trained simultaneously) and after training we can "get" the encoder (or decoder) by using get_encoder (or get_decoder). We could however also rename get_encoder -> obtain_encoder for example.

@benedict-96
Copy link
Collaborator Author

Changed names of functions:

  • get_encoder -> encoder
  • get_decoder -> decoder

@benedict-96 benedict-96 merged commit 7aa4e60 into main May 2, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants