fedroy commented 2 years ago

Reverts q-optimize/c3#220 to verify correct functioning of test notebook

lazyoracle commented 2 years ago

Notebooks are fine. The issue is not with this PR. The out of bounds error has existed for some time now. We can investigate it but we hopefully do not need to revert this PR. Reverting will unnecessarily dirty the git history.

codecov[bot] commented 2 years ago

Codecov Report

Merging #222 (526ae92) into dev (5eb8e8d) will decrease coverage by 0.10%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #222      +/-   ##
==========================================
- Coverage   74.83%   74.72%   -0.11%     
==========================================
  Files          38       38              
  Lines        5532     5516      -16     
==========================================
- Hits         4140     4122      -18     
- Misses       1392     1394       +2

Impacted Files	Coverage Δ
c3/model.py	`86.45% <100.00%> (-1.15%)`	:arrow_down:

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 5eb8e8d...526ae92. Read the comment docs.

MaxNaeg commented 2 years ago

We thought there was also a second issue with the code, but it's actually fine. Let's cancel this pull request and not revert.

lazyoracle commented 2 years ago

Yeah, in general, since we have a separate stable master and new Pull Requests only get merged into the unstable dev, it's fine to have buggy and WIP features getting merged. The earlier its merged, the better. We can always fix broken or semi-working things later on by making another PR that introduces the fix.

fedroy commented 2 years ago

great, then I'm closing this PR

q-optimize / c3

Revert "Arbitrary basis" #222

Codecov Report