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

constructors to fix 321 #348

Merged
merged 2 commits into from
Dec 6, 2023
Merged

constructors to fix 321 #348

merged 2 commits into from
Dec 6, 2023

Conversation

willow-ahrens
Copy link
Collaborator

@willow-ahrens willow-ahrens commented Dec 6, 2023

fixes #321 fixes #319

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (469006c) 75.93% compared to head (91eb280) 76.02%.

Files Patch % Lines
ext/SparseArraysExt.jl 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #348      +/-   ##
==========================================
+ Coverage   75.93%   76.02%   +0.09%     
==========================================
  Files          78       78              
  Lines        6565     6586      +21     
==========================================
+ Hits         4985     5007      +22     
+ Misses       1580     1579       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@willow-ahrens willow-ahrens merged commit 1a3555e into main Dec 6, 2023
8 checks passed
"""
Array(arr::Union{Fiber, SwizzleArray})

Construct an array from a fiber or swizzle. May reuse memory, will usually densify the fiber.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like the implementation here can't ever re-use memory; is the "may reuse memory" here so that this could be optimized later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finch stores multidimensional dense arrays as vectors, so without adding a reshaped array operator, only vectors can share memory. However, in the future I'm not sure if it will be possible to reshape arrays without creating a reshapedarray, so I thought I'd leave the possibility open. At the very least, I could add a case for dense vectors.

@willow-ahrens willow-ahrens deleted the wma/fix321 branch December 12, 2023 17:48
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.

Implement export functions for Finch Fibers UndefVarError: FinchProtocolError not defined
2 participants