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

realigning #2

Merged
merged 50 commits into from
Apr 7, 2021
Merged

realigning #2

merged 50 commits into from
Apr 7, 2021

Conversation

MrRobot2211
Copy link
Owner

Checklist
Thank you for contributing to QuTiP! Please make sure you have finished the following tasks before opening the PR.

  • Please read Contributing to QuTiP Development
  • Contributions to qutip should follow the pep8 style.
    You can use pycodestyle to check your code automatically
  • Please add tests to cover your changes if applicable.
  • If the behavior of the code has changed or new feature has been added, please also update the documentation and the notebook. Feel free to ask if you are not sure.

Delete this checklist after you have completed all the tasks. If you have not finished them all, you can also open a Draft Pull Request to let the others know this on-going work and keep this checklist in the PR description.

Description
Describe here the proposed change.

Related issues or PRs
Please mention the related issues or PRs here. If the PR fixes an issue, use the keyword fix/fixes/fixed followed by the issue id, e.g. fix qutip#1184

Changelog
Give a short description of the PR in a few words. This will be shown in the QuTiP change log after the PR gets merged.
For example:
Fixed error checking for null matrix in essolve.
Added option for specifying resolution in Bloch.save function.

BoxiLi and others added 30 commits February 17, 2021 14:58
The compiler structure is updated to prepare for the use of scheduling. No functionality is changed according to this PR, only the internal interfaces.
fix the float point inaccuracy in pulse model
decompose -> compiler
Add a dictionary that map the name of each pulse to its indices. It makes the definition of compiler more readable.
Global phase was used in the old version for computing the analytical result. But in mesolve simulation it does not make sense. We ignore the global phase gate.
Improve the readability of the code, add more docs.
When setting up pytest, some tests were not caught by pytest.
These are test functions within a test class
but has Test instead of test in their name.
There was a undetected bug the test for processor with mcsolve.
The setter should modify _params, not calling the function set_up_params. The latter needs many additional arguments.
If there is no map between the name of available control pulses and their index, the compiler function should use directly the index of control pulses. The previous implementation was wrong. Corrections have been made as well as tets.
1. Manually determine the total number of available control pulses, when pulse_dict is not given
2. If not all control pulses is used, the processor needs to handle this correctly. Hence load_circuit is modified for cavityqed.py and spinchian.py
Correct the doc string for pulse_dict;
Correct usage of **kwargs;
Separate the treatment of one pulse into a private function for readability.
Use last_pulse_time for clearer naming.
To guarantee that the first element is larger than tol and avoid
strange use of np.append.
The compile method is further split up into several small private methods
The tested versions of these imports and ancient, and our hard
installation requirements mean that we can assume these version
constraints are satisfied.
These were needless busy work; Numpy and SciPy are mandatory
dependencies, and we have hard version requirements for them at install.
It's not our fault if stuff doesn't work because the user has
deliberately bypassed their package manager.

Also, this just meant one additional place to encode the version
requirements, rather than having a single source of truth.

We don't remove version tests for the _optional_ Cython, because the
user may just have an old version installed and not have realised it was
an optional dependency.
Release versions of QuTiP don't distribute setup.py, so this test could
only return True if you've loaded a dev version with the "ISRELEASED"
flag set to True.  The __file__ magic variable discovers where the QuTiP
installation path is, not where the user's current working directory is,
so the test does not appear to do what it was intended to.

Its current behaviour isn't necessarily an error in the current
distribution process - as of right now, the "release distribution"
documentation would actually have you _try_ to run QuTiP in this state
in order to run final tests before releasing.
There were many cases in qutip.__init__ that ignored any exception
thrown, and a couple of places where `try` statements covered large
tracts of code that shouldn't have been allowed to fail.

For example, the only part of the Cython import process that is
acceptable to fail is the
    import Cython
statement (if the user doesn't have Cython, which is an optional
dependency).  If they _do_ have it, then any subsequent failure in
setting up the pyximport mechanism should be treated as an unhandled
exception.  From the user's side, they have fulfilled the requirement to
use Cython, so we shouldn't mask failures setting up the Cython
environment.  That makes it harder to debug issues for both us and the
users.

It is possible that this change will cause errors for previously working
code.  However, any issues turned up are more likely to be caused by
other factors, just we've been completely ignoring them up til now.
Previously the pyximport-specific logic was split between __init__ and
qutip.cy.pyxbuilder.  Since it's only relevant to pyximport, it should
all be in only one place, which makes the interplay bewteen different
elements easier to understand.

The CFLAGS check can be done on the first call to the pyximport
mechanism; we want to ensure that distutils is imported as late as
possible, because if anything wants to access setuptools before, we'll
get nasty warnings.  That's not as unlikely as it sounds, because Cython
may need to access it itself, and our new coefficients (coming in the
next major release) use setuptools as well.
jakelishman and others added 20 commits March 27, 2021 12:53
Remove the duplication, and simplify the branching logic so it's easier
to read.

Also OpenMP is written like that, not as OPENMP in all-caps.
We don't support Python 2, so the __future__ imports aren't necessary
any more.  sys was imported by not used.
This ensures that the OpenMP calibration code in __init__ is also tested
when running on Travis.  The tests directly call OpenMP functions, so
these should still have been tested correctly.
Fix OpenMP calibration and tidyup __init__
This doesn't change the number of tests run, just lets pytest recognise
them as different iterations.  This is helpful for determining how many
of a run have failed, and useful for spotting any patterns in the
dimensions that fail.
This complete test would have a failure about one run in every ten with
only one or two elements exceeding the tolerances on Macs when using
OpenBLAS.  These failures couldn't be reproduced when using MKL.  It is
very likely that this is related to us using `eig` instead of `eigh` in
this particular configuration (avoiding segfaults), which has a slight
effect on the tolerances, since `eig` is comparing complex tolerances
not real ones.

It should be safe to increase our tolerance marginally, because the
problem is not a logic failure in our code, but just a minor loss of
precision on one particular platform.

Fix gh-1431
The default value of pulse_dict is changed to be {} instead of None. An empty pulse_dict does not have any special use case and it should be the same as no mapping given. Setting the default to be empty will same some additional treatment of None.
Also for operator_to_vector, though this is harder to mis-use.  The
documentation now spells out exactly what we expect, and we verify that
we do have the correct inputs, rather than exploding with a misleading
error message (at best) or silently doing something very unexpected (at
worst).

Fix gh-1204.
Improve documentation of vector_to_operator
The first step of calculating the trace norm is to do
    A @ A.dag()
which we did internally in Qobj.norm.  This creates a temporary Qobj,
and those created from Qobj.__mul__ are subject to tidyup by default, so
this temporary structure may incorrectly get tidied to the zero matrix.

For example, the code
    qutip.qdiags(1e-7 * np.random.rand(10), 0).norm()
would previously give 0, clearly all elements of A @ A.dag() would be
less than 1e-14, and subject to tidyup.

We avoid the call that may be tidied up by jumping straight to CSR-CSR
matrix multiplication, and set isherm=True without going via Qobj, since
the operation is Hermitian for any matrix.

Fix gh-952
The latter had the potential to cause problems when moving to
cross-compilation for macosx-arm64 on conda-forge.  See
    https://conda-forge.org/blog/posts/2020-10-29-macos-arm64/
for more details.
Change sys.platform to sysconfig.get_platform in setup.py
I did this previously a handful of commits ago in b7df1b5, but the
tolerances chosen there weren't ideal.

This is only especially relevant for cases where we have to use generic
eigenvalue solvers in place of Hermitian ones (e.g. when the Hermitian
ones have temperamental segfaults).  I ran test_diag_liou_mult for all
dimensions from 2 to 99 inclusive 2000 times, and measured the maximum
absolute and relative tolerance for each case.  I found that 3 times out
of 2000 the absolute tolerance exceeded 1e-10, and 12 times out of 2000
the relative tolerance exceeded 1e-7.  The max values seen were 4.24e-10
and 3.12e-7 respectively.

This updates the tolerances to 1e-9 (absolute) and 1e-6 (relative),
which still seem like reasonable tolerances, and should ensure that
we're testing correctly.
Update diag_liou_mult test tolerances
Integrate scheduler into the compiler
@MrRobot2211 MrRobot2211 merged commit 9456eb4 into MrRobot2211:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants