-
Notifications
You must be signed in to change notification settings - Fork 65
Add multi table support to ResultsExplorer #505
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
Add multi table support to ResultsExplorer #505
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature_branch/mutli_table_benchmark #505 +/- ##
========================================================================
+ Coverage 77.75% 77.98% +0.23%
========================================================================
Files 30 30
Lines 2580 2621 +41
========================================================================
+ Hits 2006 2044 +38
- Misses 574 577 +3
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:
|
08ae180 to
df54c6f
Compare
df5e11c to
c51dc98
Compare
|
Hi @fealho, coud you update this folder to match the new To generate the Then you can manually change the date to create multiple examples that you can test with some integration tests. |
2da47a7 to
688666d
Compare
561b1cb to
15288cf
Compare
|
@fealho if you want to try out your changes for results saved on AWS you can use: I ran small |
5d1162b to
a985b08
Compare
9e2b28f to
4c87a30
Compare
9a97ea3 to
ba3482b
Compare
pvk-developer
left a comment
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.
LGTM! Just remove the commented line
R-Palazzo
left a comment
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.
Looking good!
I just have a few questions but I tested it locally and on aws, it's working pretty well 👍
|
|
||
| _BASELINE_BY_MODALITY = { | ||
| 'single_table': SYNTHESIZER_BASELINE, | ||
| 'multi_table': 'MultiTableUniformSynthesizer', |
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.
@fealho can you ask in the Team-Engineering channel which synthesizer we should use as the baseline for multi-table here?
Just to make sure everything is as expected. With this implementation it will be easy to update it in the future 👍
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.
LGTM!
After fixing the tests it should be good to go
Update tests Fix init
2077d3c to
5d266a8
Compare
5d266a8 to
3a78dba
Compare
CU-86b7ck0t5, Resolve #488