-
Notifications
You must be signed in to change notification settings - Fork 16
Adding Bar Graph Visualization for games/applications #24
base: main
Are you sure you want to change the base?
Conversation
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.
I ran your code on my computer and everything looks great! I checked your fidelities and they are within a similar range of the paper. Good job!
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.
This is the data from the json I used |
I used the visualization code and it no longer works. I noticed that you said that you added dual file comparison, but that wasn't the issue that I was experiencing. The issue that I had was with visualization of multiple fidelities in one json file. When you run multiple benchmark together in one call: Your code was perfectly fine, it just needed to be slightly modified to deal with multiple concurrent benchmark names in the fidelity. I hope that this comment offers clarity to my issue. |
ahh, okay I didn't know you could run multiple benchmarks at once, I didn't write this with that in mind, and your problem is that the fidelity breaks down when you have multiple different benchmarks in one json file? |
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.
I've just put a few comments inline. You'll need to git mv
the file in to the red_queen
directory. The reason lint is passing right now is that it isn't being run on the file because it's outside the red queen package.
@@ -0,0 +1,153 @@ | |||
import sys | |||
import json | |||
from turtle import color |
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.
Is turtle in the requirements list? I think this will fail unless people manually install turtle so we should add it to the requirements list https://github.com/Qiskit/red-queen/blob/main/requirements.txt so that it gets installed with red queen. That being said I don;t think anything is actually using this.
print("Invalid Data Type! Allowed parameters are: 'fidelity' and 'meantime'") | ||
if __name__ == "__main__": | ||
main(sys.argv) | ||
|
||
|
||
|
||
|
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.
print("Invalid Data Type! Allowed parameters are: 'fidelity' and 'meantime'") | |
if __name__ == "__main__": | |
main(sys.argv) | |
print("Invalid Data Type! Allowed parameters are: 'fidelity' and 'meantime'") | |
if __name__ == "__main__": | |
main(sys.argv) |
try: | ||
bench_name = {} | ||
bench_fid= {} | ||
for i,bench in enumerate(contents["benchmarks"]): | ||
bench_name[bench["name"]] = i + 1 | ||
if bench_fid.get(bench["name"]): | ||
bench_fid[bench["name"]] = bench_fid.get(bench["name"]) + bench["stats"]["quality"]["fidelity"] | ||
else: | ||
bench_fid[bench["name"]] = bench["stats"]["quality"]["fidelity"] | ||
fidavg = [] | ||
for i in bench_fid: | ||
fidavg.append(bench_fid[i]/bench_name[i]) | ||
return bench_name.keys(), fidavg | ||
except Exception as e: | ||
print("Uh, Oh! Something went wrong") | ||
print(e) |
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.
Is there a reason to just not have the exception here raise?
if len(args) == 4: | ||
viewtype = args[1] | ||
file = args[2] | ||
datatype = args[3] | ||
if len(args) == 5: | ||
viewtype = args[1] | ||
file = args[2] | ||
file2 = args[3] | ||
datatype = args[4] |
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.
Can you leverage argparse for this so we have a --help
to explain the options to users?
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.
If need be, I could help you with this @danielleodigie.
This pull request adds bar graph visualization for the applications the QxQ interns are working on. I added to the README, but to run it, after getting your results JSON, run:
For example:
Currently, I only have
fidelity
andmeantime
as available data types, but I'm open to adding more, I just wasn't sure which ones would be useful. If you have any suggestions, it would help a ton! :)Also, I noticed it didn't work as well with the mapping benchmarks, so if you can help with that, that would be awesome