Allow me to access loss values for GAN-based synthesizers #1681
Conversation
frances-h
requested review from
rwedge and
amontanez24
and removed request for
a team
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1681 +/- ##
=======================================
Coverage 97.09% 97.10%
=======================================
Files 47 47
Lines 4374 4386 +12
=======================================
+ Hits 4247 4259 +12
Misses 127 127 ☔ View full report in Codecov by Sentry. |
| pd.DataFrame: | ||
| Dataframe of loss values per epoch | ||
| """ | ||
| synthesizer = self._table_synthesizers[table_name] |
There was a problem hiding this comment.
should we error if the table name is bad? I think there is an error message we use in a lot of places for that
There was a problem hiding this comment.
I think we usually use _validate_table_name but the full error message doesn't really make sense in this context, so I used an edited version.
sdv/single_table/utils.py
Outdated
| @@ -349,3 +349,23 @@ def log_numerical_distributions_error(numerical_distributions, processed_data_co | |||
| f"cannot be applied to column '{column}' because it no longer " | |||
| 'exists after preprocessing.' | |||
| ) | |||
|
|
|||
|
|
|||
| class GANMixin: | |||
There was a problem hiding this comment.
How do you feel about putting this class in the ctgan file since all the gans are already there? Also I think VAEs aren't technically GANs. Maybe we should change the name?
There was a problem hiding this comment.
Changed to LossValuesMixin instead
Resolve #1671