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

If experiment_data.save() fails it should raise an exception #1134

Open
oliverdial opened this issue Apr 11, 2023 · 4 comments · May be fixed by #1278
Open

If experiment_data.save() fails it should raise an exception #1134

oliverdial opened this issue Apr 11, 2023 · 4 comments · May be fixed by #1278

Comments

@oliverdial
Copy link

https://github.com/Qiskit/qiskit-experiments/blob/049436a3d184adb51bb559316483cd44f1e2c925/qiskit_experiments/framework/experiment_data.py#L1339

Here; if you hit this condition the experiment data is not saved, but the function returns as if it had saved. In the context of, for example launching a bunch of experiments from a loop, this could cause a user to lose a lot of data. (or am I missing something here?)

@coruscating
Copy link
Collaborator

Users don't typically call expdata.save() and then immediately overwrite expdata with the experiment data returned from the next experiment, which is why this hasn't been surfaced as an issue so far, but I do think the handling can be improved here. Since suppress_errors is already a flag, I'm not sure why it doesn't apply to this conditional as well. @gadial what do you think?

@oliverdial
Copy link
Author

oliverdial commented Apr 11, 2023

(If you want another use case, run_qe will return with an exit code that indicates success, even if save() fails.)

@gadial
Copy link
Contributor

gadial commented Apr 17, 2023

I agree; no reason not to raise an error here if we already have the (new) suppress_errors flag.

@coruscating
Copy link
Collaborator

I put in a fix for this in #1278. Note that if you were calling save(), which is what most users do, it already raises an error when there's no service: https://github.com/Qiskit-Extensions/qiskit-experiments/blob/f33bed7e143b68bd53243c061c603bb5216c9e54/qiskit_experiments/framework/experiment_data.py#L1696-L1705

So the new fix is only for the case when someone calls save_metadata() separately to save only the metadata.

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 a pull request may close this issue.

3 participants