Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Use array of concrete eltype for efficiency in astar #1585

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jlapeyre
Copy link

@jlapeyre jlapeyre commented Aug 20, 2021

An array of eltype Integer is used in the astar algorithm. But, the elements are always only of a single concrete subtype of Integer. This PR changes the array to the narrower type. Timing the second test of astar in the test suite (applying astar to the complete graph) shows a 2x improvement in speed.

EDIT: it would be better if ones(Integer, n) threw an error, rather than arbitrarily choosing to fill with Ints. But, that ship has sailed.

jlapeyre and others added 2 commits August 20, 2021 14:18
Replaces an array of Integer with an array of a concrete
subtype of Integer.
@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

Merging #1585 (2583f80) into master (712b8d1) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 2583f80 differs from pull request most recent head 3c32020. Consider uploading reports for the commit 3c32020 to get more accurate results

@@           Coverage Diff           @@
##           master    #1585   +/-   ##
=======================================
  Coverage   99.44%   99.44%           
=======================================
  Files         106      106           
  Lines        5551     5551           
=======================================
  Hits         5520     5520           
  Misses         31       31           

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants