SDGym should be able to automatically discover SDV Enterprise synthesizers

[R-Palazzo]

Resolve #481
Resolve #491
CU-86b7b0kaa
I share this notebook to try out the feature.

[sdv-team]

Task linked: CU-86b7b0kaa SDGym - SDGym should be able to automatically discover SDV Enterprise synthesizers #481

[codecov]

Codecov Report

❌ Patch coverage is 93.84615% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.04%. Comparing base (6a3b217) to head (ec4d347).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
sdgym/benchmark.py	76.92%	3 Missing ⚠️
sdgym/utils.py	70.00%	3 Missing ⚠️
sdgym/synthesizers/generate.py	95.00%	1 Missing ⚠️
sdgym/synthesizers/sdv.py	98.18%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #489      +/-   ##
==========================================
+ Coverage   74.86%   76.04%   +1.17%     
==========================================
  Files          29       30       +1     
  Lines        2399     2371      -28     
==========================================
+ Hits         1796     1803       +7     
+ Misses        603      568      -35     

Flag	Coverage Δ
integration	61.60% <80.76%> (+1.23%)	⬆️
unit	71.02% <89.23%> (+1.66%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[R-Palazzo]

@amontanez24 @pvk-developer
Should we support using DayZSynthesizer in a benchmark?
Right now it won’t be picked up, because our discovery only looks for subclasses of BaseSingleTableSynthesizer and BaseMultiTableSynthesizer, and DayZ doesn’t inherit from those. That’s a small change to make.

Two things to consider:

• DayZSynthesizer does not have a fit() method; its SDGym wrapper should take this into account to make it work in a benchmark.
• Both the single-table and multi-table versions are named DayZSynthesizer, but based on the context we should still be able to determine which one we need I think

[pvk-developer]

@amontanez24 @pvk-developer Should we support using DayZSynthesizer in a benchmark? Right now it won’t be picked up, because our discovery only looks for subclasses of BaseSingleTableSynthesizer and BaseMultiTableSynthesizer, and DayZ doesn’t inherit from those. That’s a small change to make.

Two things to consider:

• DayZSynthesizer does not have a fit() method; its SDGym wrapper should take this into account to make it work in a benchmark.
• Both the single-table and multi-table versions are named DayZSynthesizer, but based on the context we should still be able to determine which one we need I think

If I had to write this as per the issue is saying, we should get all the synthesizers that are accessed by sdv.single_table such as:

In [3]: from sdv import single_table

In [4]: dict([(name, cls) for name, cls in single_table.__dict__.items() if isinstance(cls, type)])
Out[4]: 
{'CopulaGANSynthesizer': sdv.single_table.copulagan.CopulaGANSynthesizer,
 'GaussianCopulaSynthesizer': sdv.single_table.copulas.GaussianCopulaSynthesizer,
 'CTGANSynthesizer': sdv.single_table.ctgan.CTGANSynthesizer,
 'TVAESynthesizer': sdv.single_table.ctgan.TVAESynthesizer,
 'DayZSynthesizer': sdv_enterprise.sdv.single_table.dayz.day_zero.DayZSynthesizer,
 'BootstrapSynthesizer': sdv_enterprise.sdv.single_table.bootstrap.bootstrap.BootstrapSynthesizer,
 'DPGCFlexSynthesizer': sdv_enterprise.sdv.single_table.differential_privacy.dp_gc_flex_synthesizer.DPGCFlexSynthesizer,
 'DPGCSynthesizer': sdv_enterprise.sdv.single_table.differential_privacy.dp_gc_synthesizer.DPGCSynthesizer,
 'SegmentSynthesizer': sdv_enterprise.sdv.single_table.segment.segment.SegmentSynthesizer,
 'XGCSynthesizer': sdv_enterprise.sdv.single_table.xgc.xgc.XGCSynthesizer}

Then my other comment on how you got the subclasses can go away and you could just use this functionality instead.

[amontanez24]

I might be missing something but my main thought is that we don't need to make the classes importable. I think we can just create a class object from a string matching a valid SDV synthesizer when it is requested by the benchmark.

[pvk-developer]

LGTM!

[amontanez24]

Instead of directly returning an instance of BaselineSDVSynthesizer, can you add a function, create_sdv_synthesizer_class and in it return something like:

    CustomSynthesizer = type(
        class_name, # Should be the synthesizer name in sdv
        (BaselineSynthesizer,),
        {
            '__module__': __name__,
            'get_trained_synthesizer': get_trained_synthesizer,
            'sample_from_synthesizer': sample_from_synthesizer,
        },
    )

[R-Palazzo]

Yes done in 33e5813

[R-Palazzo]

@amontanez24 did some additional tests locally and also checked that it works on ec2 instances.
There is also this notebook to try out with sdv-enterprise.