Closed purva-thakre closed 4 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 98.19%. Comparing base (
ccb9ff6
) to head (bee9d7a
). Report is 62 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I'll be late to the Community call today due to another appointment right before.
Apologies for the delay in adding this RFC! We can discuss the proposed changes async through the Google doc comments if there's not enough time during the community call.
The plan is to leave this PR unmerged until the RFC is accepted. Anyone who would like to give comments on the RFC is welcome to during the next two (say?) weeks.
cc @unitaryfund/engineering
Andrea left a comment on the RFC that we should keep the LRE and ZNE inference methods separate. Initially, I thought it would be better to move mitiq.zne.inference
to mitiq.inference
in order to avoid defining another module for the multivariate extrapolation methods.
The refactor to the Factory
class in mitiq would have comprised of : current functions work for single variable extrapolation and there would be another option for the multivariate extrapolation. Based on Andrea's comments, this refactor would be a major breaking change.
FWIW I do agree with Andrea that this would be a major breaking change. I thought of proposing this breaking change to make it possible for us to extend multivariate extrapolation beyond Richardson extrapolation (in the near future).
I would appreciate comments from others on the two comments in the Backward Compatibility
section. I can work on making changes to the RFC afterward.
Edit: Maybe, I am missing something here. I'll try to understand why a Factory
refactor might not work for multivariate extrapolation and update the details in the RFC soon.
Andrea left a comment on the RFC that we should keep the LRE and ZNE inference methods separate. Initially, I thought it would be better to move
mitiq.zne.inference
tomitiq.inference
in order to avoid defining another module for the multivariate extrapolation methods.
I agree that it would make sense to move all the inference methods into their own module as you describe @purva-thakre .
Current version of the RFC (without the linked Jupyter notebook) was accepted during this week's community call.
Layerwise Richardson Extrapolation RFC
Fixes #2224