-
Notifications
You must be signed in to change notification settings - Fork 90
Confusing output for width and depth from resource estimator #267
Conversation
src/Simulation/Simulators/ResourcesEstimator/ResourcesEstimator.cs
Outdated
Show resolved
Hide resolved
|
Have decided that this breaking change isn't justified and will only update the documentation: |
|
The additional tests would be nice to have, though... I'll update them to use the old statistics names. |
msoeken
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.
Can we print a warning when calling ToTSV() that the depth and width metrics are not compatible to each other. It's possible that not everyone is aware of this when just calling the function without checking the documentation.
|
Warning messages like this become annoying very quickly because the user learns the fact after seeing the warning once or twice but the message would keep showing up, complicating the analysis of the output and demanding attention. I am also worried to proliferate hard-coded text messages through the codebase as that is against the globalization guidelines. Arguably, if a user is confused by what they are seeing in the statistics, that would be the perfect time for them to check the documentation. Basically, I believe we should keep code and documentation separate. I am not resolving #192, please feel free to add more comments there if you think additional fixes could improve the situation. For this change I'd like to go ahead and add the tests -- would you mind unblocking the PR? |
* Added tests to clarify that width/depth statistics reflect lower bounds
Related to #192
The resource estimator currently calculates independent lower bounds for width and depth metrics. When reported next to each other they might look confusing because it might not be possible to satisfy both bounds at the same time.
While we consider whether it's worth implementing the full optimization logic in the resource estimator (so it could calculate corelated width/depth metrics), this change renames the current metrics to WidthLowerBound and DepthLowerBound to communicate the intent more clearly.
Have decided that this breaking change isn't justified and will only update the documentation:
MicrosoftDocs/quantum-docs-pr#930