Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix scc bug #31

Merged
merged 1 commit into from
Jan 8, 2020
Merged

Fix scc bug #31

merged 1 commit into from
Jan 8, 2020

Conversation

corakingdon
Copy link
Collaborator

@corakingdon corakingdon commented Dec 24, 2019

I'm really surprised we didn't notice this sooner, but for a while our compute_scc function has been returning completely incorrect numbers because of this (it was returning -$0.027 instead of $37 for year=2015, prtp-0.03, eta=0).

For reference, I think it was this commit a while ago that broke it: 6cf264d

@codecov
Copy link

codecov bot commented Dec 24, 2019

Codecov Report

Merging #31 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #31   +/-   ##
======================================
  Coverage    88.7%   88.7%           
======================================
  Files          12      12           
  Lines         248     248           
======================================
  Hits          220     220           
  Misses         28      28
Impacted Files Coverage Δ
src/marginaldamage.jl 67.39% <100%> (ø) ⬆️

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 713a482...488c698. Read the comment docs.

@davidanthoff davidanthoff merged commit 5125c87 into master Jan 8, 2020
@davidanthoff davidanthoff deleted the scc-bug branch January 8, 2020 22:20
@davidanthoff
Copy link
Member

Can we add a test that makes sure this doesn't happen again?

@davidanthoff davidanthoff mentioned this pull request Jan 8, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants