Visualization #62
Visualization #62
Conversation
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
stroblme
requested review from
a team,
eileen-kuehn and
cDenius
and removed request for
a team
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
stroblme
requested review from
a team
and removed request for
eileen-kuehn,
cDenius and
a team
stroblme
requested review from
eileen-kuehn and
cDenius
and removed request for
a team
| @@ -28,42 +28,48 @@ class design: | |||
| qual_main = px.colors.qualitative.Dark2 # set1 | |||
| qual_second = px.colors.qualitative.Pastel2 # pastel1 | |||
|
|
|||
| seq_main = px.colors.sequential.Viridis # pastel1 | |||
| seq_main = px.colors.sequential.thermal # pastel1 | |||
There was a problem hiding this comment.
What does the "pastel1" comment mean?
There was a problem hiding this comment.
added more descriptive comment, will resolve with #85
| @@ -111,6 +117,145 @@ def extract_framework_name_from_id(identifier): | |||
| return identifier.replace("fw", "").capitalize().replace("_", " ") | |||
|
|
|||
|
|
|||
| def heatmap_viz(x, y, z, z_title, x_title, log_x, y_title, log_y, plot_title): | |||
There was a problem hiding this comment.
It might be helpful to add the data types for the different parameters. Just from reading the code I am currently guessing, that log_x and log_y might be boolean.
| plot_title=f"{framework_name} @ {q} Qubits: " | ||
| f"Circuit Depth and # of Shots", |
There was a problem hiding this comment.
Usually one needs plots without a title (e.g. for publications). So you might consider naming the plots accordingly, so that one is free to add a title with a preferred tool or to also have a flag to decide on how the title should be handled.
There was a problem hiding this comment.
agree, propose to resolve that in a separate PR -> created issue #84
| # those two color sets are well suited as they correspond regarding | ||
| # their color value but differ from their luminosity and saturation values | ||
| main_colors_it = iter(design.qual_main) | ||
| sec_colors_it = iter(design.qual_second) |
There was a problem hiding this comment.
You are using the color sets at different places, so you might consider having a function to get your current color set.
There was a problem hiding this comment.
the idea here is to have a class where the colors are specified and then just refer to the corresponding name later in the code. But I would suggest to move those "hard-coded" parameters to kedro parameters anyway. Thus I propose resolving this in a separate PR -> created issue #85
Co-authored-by: Eileen Kuehn <eileen.kuehn@kit.edu>
Co-authored-by: Eileen Kuehn <eileen.kuehn@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
Signed-off-by: lc3267 <melvin.strobl@kit.edu>
|
Thanks for your review @eileen-kuehn . Fixed some of the issues mentioned in the comments but also shifted some to be resolved in other PRs/Issues because it would introduce further modifications of pipeline+parameters which I would like to see in a separate, clean PR |
This PR fixes logscales in heatmaps (closes #49) by applying
log_2andlog_10on the axis data respectively and removes redundant code in the visualization node.It further introduces some recommendations for title and axis labeling.
Also Plotly version was bumped to the latest one available.
Depends on #61