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

Compatibility fixes to prior versions #34

Merged
merged 21 commits into from
Mar 2, 2021
Merged

Conversation

GlaserN
Copy link
Collaborator

@GlaserN GlaserN commented Feb 1, 2021

Changes include:

  • corrected behavior of flux tuning device
  • streamline noise devices
  • quantity object can be used as an argument for numpy functions, returning a np.array
  • update the model if model parameters were changed
  • more continous reordering of dressed states
  • ...

This pull request also greatly improves perfomance,

  • by using vectorized performance
  • providing a response function utilising fft instead of manual multiplication
  • using tf.constants instead of tf.variables, where it is not explicitly necessary

Fix ordering issues
test for tunable coupler signal
engineering notation fix
mute windows
replace_logdir capable of making new dir
do not give NaN grad to optimizer
optimize nice printing
@GlaserN GlaserN requested a review from fedroy February 1, 2021 16:24
@lazyoracle
Copy link
Member

Hi @GlaserN, please check out the CONTRIBUTING.md, on how to set up the pre-commit hooks. That should auto format and lint your commits and will pass the Code Formatting and Linting check that's currently failing.

@codecov-io
Copy link

codecov-io commented Feb 1, 2021

Codecov Report

Merging #34 (f50fcda) into dev (50052a5) will decrease coverage by 0.09%.
The diff coverage is 67.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #34      +/-   ##
==========================================
- Coverage   46.92%   46.82%   -0.10%     
==========================================
  Files          35       35              
  Lines        4492     4574      +82     
==========================================
+ Hits         2108     2142      +34     
- Misses       2384     2432      +48     
Impacted Files Coverage Δ
c3/libraries/envelopes.py 36.87% <0.00%> (ø)
c3/optimizers/sensitivity.py 0.00% <ø> (ø)
c3/system/tasks.py 45.28% <0.00%> (ø)
c3/libraries/fidelities.py 18.29% <11.11%> (ø)
c3/utils/utils.py 65.47% <45.45%> (-2.10%) ⬇️
c3/utils/tf_utils.py 42.57% <51.35%> (-1.87%) ⬇️
c3/optimizers/optimizer.py 74.43% <53.84%> (-1.76%) ⬇️
c3/generator/devices.py 68.44% <71.42%> (-3.26%) ⬇️
c3/system/model.py 79.71% <75.00%> (ø)
c3/c3objs.py 79.13% <82.75%> (-0.44%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50052a5...f50fcda. Read the comment docs.

@lazyoracle lazyoracle self-requested a review February 1, 2021 18:44
lazyoracle
lazyoracle previously approved these changes Feb 1, 2021
@lazyoracle lazyoracle added this to the 1.2.1 milestone Feb 1, 2021
@lazyoracle
Copy link
Member

lazyoracle commented Feb 2, 2021

Does this address #28 ?

@GlaserN
Copy link
Collaborator Author

GlaserN commented Feb 2, 2021

Does this address #28 ?
No it does not. I guess a change there would need a proper discussion.

@fedroy
Copy link
Collaborator

fedroy commented Feb 9, 2021

@lazyoracle after merging dev into the PR it fails the build (which was previously successful) and it seems to be some qiskit import. Could you give it a look?

@lazyoracle
Copy link
Member

Fixed in #48 and merged to dev. Let me know if it still persists once you merge dev into further_fixes.

@lgtm-com
Copy link

lgtm-com bot commented Mar 2, 2021

This pull request introduces 1 alert when merging 1b6e1c2 into a4ab975 - view on LGTM.com

new alerts:

  • 1 for Unused exception object

@fedroy fedroy merged commit 39ae727 into q-optimize:dev Mar 2, 2021
@lgtm-com
Copy link

lgtm-com bot commented Mar 2, 2021

This pull request introduces 1 alert when merging 6e679ad into a4ab975 - view on LGTM.com

new alerts:

  • 1 for Unused exception object

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.

4 participants