Skip to content

Add FieldMatrix: a rank-two tensor generalisation of FieldVector #626

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

Merged
merged 7 commits into from
Jul 12, 2019
Merged

Add FieldMatrix: a rank-two tensor generalisation of FieldVector #626

merged 7 commits into from
Jul 12, 2019

Conversation

CFBaptista
Copy link
Contributor

FieldVector already facilitates the creation of vector types which can be used inside Vector{<:FieldVector} arrays to represent vector fields. I also have the need to represent rank-two tensor fields (gradients of vector fields for example). Therefore, I have implemented the FieldMatrix type to facilitate this. Others might find this useful too.

Inheriting from this type makes it easy to create your own rank-two N x M tensor types in a similar fashion that FieldVector does for vectors types. Here is an example:

struct Stress <: FieldMatrix{3, 3, Float64}
    xx::Float64
    xy::Float64
    xz::Float64
    yx::Float64
    yy::Float64
    yz::Float64
    zx::Float64
    zy::Float64
    zz::Float64
end

Tests can be found in test/FieldMatrix.jl. These tests mimick the tests from test/FieldVector.jl. All tests from test/runtests.jl (with test/FieldMatrix.jl included) passed without any warning/error.

Please let me know whether this is useful and/or improvements are needed before this can be accepted. For example, I'm not too sure about the constructors I provided. They work, but might not yet be optimal performance-wise.

@tkoolen
Copy link
Contributor

tkoolen commented Jul 9, 2019

Should we generalize things more and have a FieldArray instead?

@CFBaptista
Copy link
Contributor Author

@tkoolen that would indeed support all use cases. People from solid mechanics, for example, sometimes use rank-four stiffness tensors. However, performance for large static arrays is no good, right?

@coveralls
Copy link

coveralls commented Jul 9, 2019

Coverage Status

Coverage increased (+0.01%) to 81.734% when pulling cae2d1f on cfbaptista:FieldMatrix into b150a8c on JuliaArrays:master.

@fredrikekre
Copy link
Member

struct Stress

People from solid mechanics, for example, sometimes use rank-four stiffness tensors.

Completely off-topic, but have you seen https://github.com/KristofferC/Tensors.jl ?

@c42f
Copy link
Member

c42f commented Jul 9, 2019

This looks pretty neat. Agreed that it would be preferable to generalize to arbitrary dimensions if you're willing to take a go at that. In that case I imagine FieldMatrix would become a type alias of FieldArray.

Somewhat tangential, but have you seen https://github.com/JuliaDiffEq/LabelledArrays.jl ? It's an interesting alternative approach where the array type is identified by the names of the fields. (LabelledArrays wasn't possible when we originally created StaticArrays.FieldVector but became workable with the introduction of getproperty.)

@CFBaptista
Copy link
Contributor Author

@fredrikekre I have now :-)

I had a quick glance and it seems that https://github.com/KristofferC/Tensors.jl supports at least all of my needs. Please correct me if I'm wrong, but I think types which inherit from FieldMatrix are more generic than tensors from Tensor.jl because:

  1. Tensor.jl only supports tensors with the same length in each dimension (i.e. N, N x N and N x N x N x N)
  2. The length in each dimension can only be 1, 2 or 3.
  3. Tensor.jl only supports real-valued tensors (T <: Real)

These limitations do not bother me because I don't need more, but I don't think this applies to everyone. Also I noticed strange behaviour when adding a literal value to a Tensors.Tensor:

using Tensors
T = Tensor{2, 3, Float64}(rand(3, 3))
U = T .+ 1.0

println(typeof(T))
Tensor{2,3,Float64,9}

println(typeof(U))
Array{Float64,2}

I do not like that the result is heap allocated.

@CFBaptista
Copy link
Contributor Author

@c42f Thanks and I also agree that it would be good to generalise to FieldArray. Both FieldVector and FieldMatrix can be typealises.

https://github.com/JuliaDiffEq/LabelledArrays.jl looks very promising indeed. I would be very interested in some benchmarks with functions from LinearAlgebra. It appears to be built on top on StaticArrays, so I'm sure it's performant.

@CFBaptista
Copy link
Contributor Author

As of commit 89f40d8 FieldArray has been added and FieldVector and FieldMatrix are defined as abstract types which are subtypes of StaticArray. Inheriting from FieldArray makes it possible to define your own tensor types of arbitrary rank.

The constructors might need some additional work and some actual testing of higher-rank tensors should be added.

Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

Thanks for the update, that's a great generalization for basically the same amount of code.

One issue we need to sort out is column vs row major ordering in constructors and field naming in the examples. We can demonstrate some current inconsistencies with the Stress example:

julia> struct Stress <: FieldMatrix{3, 3, Float64}
          xx::Float64
          xy::Float64
          xz::Float64
          yx::Float64
          yy::Float64
          yz::Float64
          zx::Float64
          zy::Float64
          zz::Float64
       end

# Unfortunately, constructing `Stress` strongly suggests row major ordering of elements
# but that's not what we get:
julia> Stress(1,2,3,
              4,5,6,
              7,8,9)
3×3 Stress:
 1.0  4.0  7.0
 2.0  5.0  8.0
 3.0  6.0  9.0

# Indexing at row 1, col 2 returns a surprising result compared to the field ordering in the constructor:
julia> Stress(1,2,3,
       4,5,6,
       7,8,9)[1,2]
4.0

# This is also inconsistent with accessing the second field using `.xy`:
julia> Stress(1,2,3,
       4,5,6,
       7,8,9).xy
2.0

StaticArrays assumes that arrays are stored in column major order (as does the getindex implementation via getfield). This means you need to flip the order of the fields in your example tensors if you want consistency between getindex(a, i,j) and the field names.

Here's one possible way forward:

  • Document that FieldArray should have column major field ordering unless people supply their own getindex and setindex.
  • Disable the default constructor for FieldArrays so that people can't make the row major formatting mistake I've got above.
  • Finally address Inconsistencies with multi-argument constructors #518 by inventing some syntax to allow writing general StaticArray literals. A constructor-like macro will probably be required.

Alternatively, we could

  • Document that FieldArray should have column major field ordering, unless people supply their own getindex and setindex.
  • Create a constructor which transposes the input elements so that row major input ordering turns into column major data. But honestly this feels like it could be problematic and cause more issues in the long run than it solves. Again, see Inconsistencies with multi-argument constructors #518 for some of the difficulties we already have with constructors.

@c42f
Copy link
Member

c42f commented Jul 10, 2019

Some relevant thoughts about our constructor woes over at #518 (comment).

With that proposal, the way to efficiently and unambiguously construct Stress (without users needing to define their own constructor) would be

Stress(@SA[1 2 3;
           4 5 6;
           7 8 9])

@CFBaptista
Copy link
Contributor Author

CFBaptista commented Jul 10, 2019

Thanks for pointing this out @c42f, the order of the fields in the example in OP was an oversight of mine. It should have been:

struct Stress <: FieldMatrix{3, 3, Float64}
    xx::Float64
    yx::Float64
    zx::Float64
    xy::Float64
    yy::Float64
    zy::Float64
    xz::Float64
    yz::Float64
    zz::Float64
end

The same modification should also be applied to Stiffness in the example for FieldArray.

I would not go the column-major way because row-major is the standard / more common in Julia. We could explicitly mention it in the documentation, however, I think column-major is expected behaviour.

I fail to see the need for the macro in constructing an object of type Stress. We can already do this:

Stress(rand(3, 3))

EDIT

I meant to say column-major instead of row-major and vice versa.

@c42f
Copy link
Member

c42f commented Jul 10, 2019

Excellent, thanks for sorting out the field ordering.

I would not go the column-major way because row-major is the standard / more common in Julia. We could explicitly mention it in the documentation, however, I think column-major is expected behaviour.

Column major is expected by the subset of Julia users who know to think about storage orders. However, the average user of multidimensional arrays would generally not need to know about this so it's worth mentioning in the doc strings.

Stress(rand(3, 3))

This makes the compiler generate rather slow code compared to Stress(@SArray(rand(3,3))) because it forces the allocation of a temporary Matrix to hold the result from rand(3,3), followed by copying that into Stress:

julia> using StaticArrays, BenchmarkTools
[ Info: Recompiling stale cache file /home/tcfoster/.julia/compiled/v1.1/StaticArrays/yY9vm.ji for StaticArrays [90137ffa-7385-5640-81b9-e52037218182]

julia> struct Stress <: FieldMatrix{3, 3, Float64}
           xx::Float64
           yx::Float64
           zx::Float64
           xy::Float64
           yy::Float64
           zy::Float64
           xz::Float64
           yz::Float64
           zz::Float64
       end

julia> @btime Stress(rand(3,3))
  46.995 ns (1 allocation: 160 bytes)
3×3 Stress:
 0.592865  0.26077   0.870697
 0.900356  0.129086  0.6465  
 0.157575  0.422919  0.219663

julia> @btime Stress(@SArray(rand(3,3)))
  17.234 ns (0 allocations: 0 bytes)
3×3 Stress:
 0.20502   0.166018  0.699433
 0.790852  0.96874   0.207043
 0.292533  0.03907   0.831171

For cases where there's less work being done compared to generating random numbers this will only be more pronounced.

@CFBaptista
Copy link
Contributor Author

CFBaptista commented Jul 10, 2019

Yes, I meant column-major instead of row-major and vice versa. I edited my post.

Ah I didn't know it was slow. Then I agree with the macro as well.

@c42f
Copy link
Member

c42f commented Jul 10, 2019

By the way, I hope you can you see what I mean about the perils of the multi-argument constructor? With the latest version:

julia> using StaticArrays

julia> struct Stress <: FieldMatrix{3, 3, Float64}
           xx::Float64
           yx::Float64
           zx::Float64
           xy::Float64
           yy::Float64
           zy::Float64
           xz::Float64
           yz::Float64
           zz::Float64
       end

julia> Stress(1,2,3,  # Column major ordering with misleading formatting
              4,5,6,
              7,8,9)
3×3 Stress:  # Oops, interpreted as the transpose of the above
 1.0  4.0  7.0
 2.0  5.0  8.0
 3.0  6.0  9.0

This isn't your fault! The problem of constructors is a difficult problem we've been struggling with for some time (see #518).

However I think it's worth mentioning this potential pitfall somewhere. It would be possible to disallow the multi-argument constructor by default in an effort to prevent these issues though that might be heavy handed. It would be less bad with a shorter @SA macro as proposed in #518.

@CFBaptista
Copy link
Contributor Author

Yes, I do see the issue with the multi-argument constructor. I will make a mention of this in the documentation.

I'm in for @SA. It's short and it saves memory.

However, Stress is a struct with 9 fields, so I don't know how you want to disallow a constructor with 9 arguments. This might be a Julia feature I am not aware of.

Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

I've left some suggestions for the documentation. Other than that, I think this is basically good to go.

Thanks for putting in the effort to make this a really solid contribution :-)

@tkoolen
Copy link
Contributor

tkoolen commented Jul 11, 2019

I'm with @c42f, thanks very much for the effort!

@c42f
Copy link
Member

c42f commented Jul 11, 2019

However, Stress is a struct with 9 fields, so I don't know how you want to disallow a constructor with 9 arguments. This might be a Julia feature I am not aware of.

Well anything is possible with an inner constructor but that doesn't help us here because any general constructors we define for FieldArray must be an outer constructor. I think we can revisit this later if we feel the need, perhaps as part of deciding exactly what to do with #518.

@c42f c42f merged commit 943a896 into JuliaArrays:master Jul 12, 2019
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.

5 participants