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

Redesign defensive code #107

Open
gefux opened this issue Jan 27, 2024 · 2 comments
Open

Redesign defensive code #107

gefux opened this issue Jan 27, 2024 · 2 comments
Assignees
Labels
tidy up Something needs to be tidied up

Comments

@gefux
Copy link
Member

gefux commented Jan 27, 2024

Currently the code checks the type of most input parameters given by the user. Though this might not be very Pythonic it is certainly useful to catch non-sensical input early on and provide the user with a helpful error message.
However, this input checking is not done consistently and leads to a lot of boiler plate code. This should be made consistant and avoid boilerplate.

@gefux gefux added the tidy up Something needs to be tidied up label Jan 27, 2024
@gefux gefux added this to the version 0.5 milestone Jan 27, 2024
@gefux gefux self-assigned this Jan 27, 2024
@piperfw piperfw mentioned this issue May 5, 2024
15 tasks
@piperfw
Copy link
Collaborator

piperfw commented May 5, 2024

A related point I found is that our use of optional arguments also seems inconsistent: we have optional arguments for internal functions that are always specified in the code (albeit sometimes with the special value None). I think we will always want to specify them in the backend code to be explicit, i.e., there is little reason for them to be optional.

degeneracy_maps in BaseTempoBackend is an example.

@piperfw
Copy link
Collaborator

piperfw commented Jun 2, 2024

Small bug that should be addressed with this issue.

If one provides a callable that returns a string rather than a float in the gammas argument of a System, _check_tdependent_gammas_linblad_operators passes but the actual calculation fails with a numpy error.

This is because _check_tdependent_gammas_linblad_operators just tries to convert the gamma to a float (which it can for a string) rather than assert that is the actual return type. There are likely other instances of this flaw in the code.

@gefux gefux removed this from the version 0.5 milestone Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tidy up Something needs to be tidied up
Projects
None yet
Development

No branches or pull requests

2 participants