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

Fix nla allocation situation #125

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

Conversation

Chronum94
Copy link

This is an attempt at fixing #120. Preemptive apologies if the code isn't up to best practices, I just saw the issue lying around and tried to hack together a solution.

I'm on Julia 1.7.3, so there are some tests failing ( #124 ), but the same tests fail before and after, so at least we have that going.

The speedups are most noticeable for the case of many calls with small arrays, with ~1.2-1.4x speedup for 1k x 1k arrays. I don't know what the arrays are supposed to look like, so I just tested out a bunch of sizes.

I'm not really a common Julia package contributor, so I can attempt edits if there's something I've done horrendously wrong in terms of style/best practices.

@MartinuzziFrancesco
Copy link
Member

thanks for taking a stab at this, at a first glance it seems to be on point! the nla functions are explicitly tested so as long as those tests pass we should be good to go

@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #125 (20bc3ae) into master (479c82d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
+ Coverage   87.33%   87.35%   +0.02%     
==========================================
  Files          13       13              
  Lines         616      617       +1     
==========================================
+ Hits          538      539       +1     
  Misses         78       78              
Impacted Files Coverage Δ
src/states.jl 94.00% <100.00%> (+0.12%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

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.

None yet

2 participants