Skip to content
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

Harmonize remote and local options & bump to 0.5.0 #23

Merged
merged 3 commits into from
Nov 30, 2023

Conversation

MaxleDrut
Copy link

Description: (copied from commit)
When we introduced the local provider, we made it so we declare its options (nb_photons, kappa_1...) when using get_backend().

This is contradictory to what is happening in the remote provider, where options must be passed in execute().

This commit harmonizes this behaviour by allowing to set the options in get_backend() for the remote provider. (which actually makes some sense to do it like this when we want to set the nb_photons for example).
This allows to quickly swap between the local and remote provider.

We aim to stay as permissive as possible, by still letting the user passing options at execute() for the remote provider.

Safe to revert.

@MaxleDrut MaxleDrut force-pushed the harmonize-remote-local branch from ca7c256 to 14ec6e2 Compare November 29, 2023 11:07
@MaxleDrut MaxleDrut requested a review from jrj-d November 29, 2023 11:12
Maxence Drutel added 3 commits November 29, 2023 16:01
When we introduced the local provider, we made it so we declare
its options (nb_photons, kappa_1...) when using get_backend().

This is contradictory to what is happening in the remote provider,
where options must be passed in execute().

This commit harmonizes this behaviour by allowing to set the options
in get_backend() for the remote provider.
(which actually makes some sense to do it like this when we want to
set the nb_photons for example).
This allows to quickly swap between the local and remote provider.

We aim to stay as permissive as possible, by still letting the user
passing options at execute() for the remote provider.
This is a fix where we found out the "kappa1" parameter of targets
was not converted to "kappa_1" in snake_case when getting a backend as
it should.

This came from our home made to_snake_case and to_camel_case functions
using some obscure regex. Instead, this commit replaces these functions
with Pydantic's, that are more tested and proven.
@MaxleDrut MaxleDrut force-pushed the harmonize-remote-local branch from 14ec6e2 to 6682188 Compare November 29, 2023 15:05
Copy link
Contributor

@jrj-d jrj-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR! Don't forget to update the doc and Laurent's changelog for 5.0.0

@MaxleDrut MaxleDrut merged commit 1ffebe7 into main Nov 30, 2023
16 checks passed
@MaxleDrut MaxleDrut deleted the harmonize-remote-local branch November 30, 2023 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants