Codecov Report

Merging #1568 (f05cdea) into master (1769686) will increase coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head f05cdea differs from pull request most recent head e2b5f8b. Consider uploading reports for the commit e2b5f8b to get more accurate results

@@           Coverage Diff           @@
##           master    #1568   +/-   ##
=======================================
  Coverage   99.44%   99.44%           
=======================================
  Files         106      106           
  Lines        5551     5553    +2     
=======================================
+ Hits         5520     5522    +2     
  Misses         31       31           

Jun 04 '21 11:06 codecov[bot]

cc @jpfairbanks or @simonschoelly for a quick review please. Thanks.

Jun 04 '21 11:06 sbromberger

What about the case where sigma is negative because of integer arithmetic wrap around? Do you still want 0 closeness?

Jun 05 '21 00:06 jpfairbanks

Good point. I wonder if it makes sense to check against 0.0 and the again against < 0.0 and throw an IntegerOverflowError or something.

Jun 05 '21 00:06 sbromberger

An extra branch that won't get hit except in exceptional circumstances seems worth it to me. Well, can we prove that this can't overflow because it is a sum of distances and the distances are bounded by nv(g)? I guess for Int16 overflow is a real possibility if you have a path graph?

Jun 05 '21 12:06 jpfairbanks

Actually, I don't see how we even get to σ == 0. We explicitly check to see whether the degree is zero in line 13, and by definition since we're using Dijkstra any edge must have a positive nonzero weight. What's really going on here?

Jun 05 '21 12:06 sbromberger