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

WIP: Unitful arrays #191

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

Conversation

cstjean
Copy link
Contributor

@cstjean cstjean commented Nov 14, 2018

I believe that the datastructure is fine, but I am unhappy with everything else. My use case is with StaticArrays, and there doesn't seem to be a nice way of accommodating both Array and SArray objects in the unit conversion routines. At least, not without also requiring StaticArrays, and at that point, we should probably start a new package entirely. Suggestions are welcome.

Update: it seems that there's a solution to this problem after all. To be implemented.

cc. @mschauer

@coveralls
Copy link

coveralls commented Nov 14, 2018

Coverage Status

Coverage decreased (-0.1%) to 74.398% when pulling 3cdd893 on cstjean:unitful-arrays into c76f881 on ajkeller34:master.

@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

Merging #191 into master will decrease coverage by 0.07%.
The diff coverage is 77.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #191      +/-   ##
==========================================
- Coverage   74.65%   74.58%   -0.08%     
==========================================
  Files          14       15       +1     
  Lines         947      968      +21     
==========================================
+ Hits          707      722      +15     
- Misses        240      246       +6
Impacted Files Coverage Δ
src/Unitful.jl 100% <ø> (ø) ⬆️
src/arrays.jl 77.27% <77.27%> (ø)
src/logarithm.jl 66.5% <0%> (-0.66%) ⬇️

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 c76f881...3cdd893. Read the comment docs.

@alhirzel
Copy link

I recently started to encounter issues with arrays of Unitful values, and wanted to present one example. @cstjean, do you think this would be addressable?

# fit a quadratic to three points, y = 5x^2 + 0.1, x in meters, y in seconds
using Unitful.DefaultSymbols

x = [0.0s  1.0s   2.0s]'
y = [0.2m  5.2m  20.2m]'

A = [x[1]^2 x[1]^1 x[1]^0
     x[2]^2 x[2]^1 x[2]^0
     x[3]^2 x[3]^1 x[3]^0]

# ideally, units would be calculated rather than assigned here
coeffs = ustrip.(A)\ustrip.(y) .* [m/s^2; m/s; m]
@assert all(ustrip.(abs.(coeffs - [5.0m/s^2; 0.0m/s; 0.2m])) .< 1.0e-12)
coeffs

@cstjean
Copy link
Contributor Author

cstjean commented Mar 22, 2019

It's should be doable with another special array type, but this PR only supports units of the form

[a*x a*y a*z
 b*x b*y b*z
 c*x c*y c*z]

There might already be a package that generates arrays of the form your A has.

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

3 participants