Add multi-table UniformSynthesizer #497
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature_branch/mutli_table_benchmark #497 +/- ##
========================================================================
+ Coverage 76.04% 76.39% +0.35%
========================================================================
Files 30 30
Lines 2371 2411 +40
========================================================================
+ Hits 1803 1842 +39
- Misses 568 569 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
R-Palazzo
force-pushed
the
issue-485-add-uniformsynthesizer
branch
from
84f1f71 to
6d659cf
Compare
R-Palazzo
commented
Nov 18, 2025
R-Palazzo
changed the base branch from
main
to
feature_branch/mutli_table_benchmark
fealho
reviewed
Nov 18, 2025
Member
fealho
left a comment
fealho
left a comment
There was a problem hiding this comment.
Looks good, I only left 1 question.
amontanez24
requested changes
Nov 20, 2025
amontanez24
approved these changes
Nov 24, 2025
Contributor
amontanez24
left a comment
amontanez24
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the changes
| model.fit(data) | ||
|
|
||
| return model | ||
| self._internal_synthesizer = model |
Contributor
There was a problem hiding this comment.
we could just call it self._model
pvk-developer
approved these changes
Nov 25, 2025
fealho
approved these changes
Nov 25, 2025
Member
fealho
left a comment
fealho
left a comment
There was a problem hiding this comment.
Left a couple of minor docstring comments, besides that it looks good to me 👍
R-Palazzo
merged commit f5abf35
into
feature_branch/mutli_table_benchmark
44 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolve #485
CU-86b7chzve
In this PR:
1 - I introduce a
_fit()method in theBaseSynthesizerclass. This method is similar to what we do in SDV; it fits all synthesizer parameters that need to be learned from the data and metadata.2 - I refactor
get_trained_synthesizerso it always returns an SDGym synthesizer object.3 - For SDV and third-party synthesizers, I defined the
_internal_synthesizerattribute, which references the actual SDV or third-party model that the SDGym synthesizer wraps around.4 - Define a
MultiTableBaselineSynthesizerto use thescaleparameter when sampling data.@amontanez24, @pvk-developer, @fealho, I ran an AWS test today from this branch with Variant to ensure everything still works as expected with these changes. The result is here