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 on using Tensors #27

Merged
merged 4 commits into from
Nov 26, 2019
Merged

wip on using Tensors #27

merged 4 commits into from
Nov 26, 2019

Conversation

KristofferC
Copy link
Contributor

@KristofferC KristofferC commented Sep 14, 2018

Still quite WIP because there is quite a lot of churn to go through when changing this.

I believe something like this is a good way forward. It will take a while to push through but in the end it should be worth it.

Changing

function jacobian(B, X, xi)
    dB = eval_dbasis(B, xi)
    dim1, nbasis = size(B)
    dim2 = length(first(X))
    J = zeros(dim1, dim2)
    for i=1:dim1
        for j=1:dim2
            for k=1:nbasis
                J[i,j] += dB[i,k]*X[k][j]
            end
        end
    end
end

to

function jacobian(B::AbstractBasis, X::Vector, xi)
    dB = eval_dbasis(B, xi)
    J = zero(Tensor{2, dim(B)})
    for i = 1:length(X)
        J += dB[i] ⊗ X[i]
    end
    return J
end

is a pretty good example I feel.

@TeroFrondelius
Copy link
Member

Yes definitely. I talked with @ahojukka5 today and we believe there is a big potential performance gain in this approach.

@KristofferC
Copy link
Contributor Author

For multithreading we really need to reduce allocations to a minimum. When Julia's GC run, all threads stop so allocating is extra bad from a multithreading point of view. So starting to use these type of static data structures will help with that I feel.

@TeroFrondelius
Copy link
Member

We originally made a decision that we should stick in ascii characters. I think now is the correct time to review our decision. Here the special operator syntax really helps readability.

@KristofferC
Copy link
Contributor Author

I can change it to otimes(dB[i], X[i]) but I personally kind of like it. As long as it isn't forced upon users it can perhaps be OK.

@ahojukka5
Copy link
Member

I like otimes more.

@KristofferC
Copy link
Contributor Author

KristofferC commented Sep 15, 2018

Ok, will change to that then.

And instead of

ϵ_i ⊡ C ⊡ ϵ_j

we do

dcontract(epsilon_i, dconstract(C, epsilon_j))

I guess.

@ahojukka5
Copy link
Member

The problem with utf-8 characters, in general, is that people do not know how to make them. I did find someone using nabla in her code, and when I asked how did she do that character, the answer was that she copypasted it everytime from somewhere else (did not know about \nabla + tab). My opinion is that utf-8 characters look cool but there are more downsides than benefits when using them.

@KristofferC
Copy link
Contributor Author

KristofferC commented Oct 3, 2018

I updated this to use Tensors throughout and I rewrote some things just how I would have done it. It won't be directly usable with the rest of JuliaFEM but perhaps it can be used for some inspiration in using Tensors.jl in the future.

As a performance comparison:

Before

b = BasisInfo(Quad4)
X = ((0.0,0.0), (1.0,0.0), (1.0,1.0), (0.0,1.0))
xi = (0.0, 0.0)
@btime eval_basis!($b, $X, $xi)
  203.700 ns (0 allocations: 0 bytes)

After

b = BasisInfo(Quad4)
X = Vec.([(0.0,0.0), (1.0,0.0), (1.0,1.0), (0.0,1.0)])
xi = Vec(0.0, 0.0)
@btime eval_basis!($b, $X, $xi)
  38.464 ns (0 allocations: 0 bytes)

I still haven't integrated the manifold + curve thing for the Jacobian since I am a bit unexperienced with that.

@ahojukka5
Copy link
Member

Would it be more Julia-style to use macro instead of create_and_eval?

@KristofferC
Copy link
Contributor Author

Maybe, you aren't really doing any syntax transformations here though so perhaps not? I dont think it matters so much, I like to avoid macros as much as possible though since you cant really know what they do unless you carefully read the docs for them.

@KristofferC
Copy link
Contributor Author

Added a few more performance tweaks, shaved of 10 ns.

@ahojukka5
Copy link
Member

For 2d surfaces in 3d space we need Jacobian to determine the normal direction. (I'm not sure is this terminology right.) Then detJ is the size factor needed for numerical integration.

In practice:

using FEMBase, FEMBasis
X = Dict(1=>[0.0,0.0,0.0], 2=>[1.0,0.0,1.0], 3=>[1.0,1.0,1.0], 4=>[0.0,1.0,0.0])
element = Element(Quad4, (1, 2, 3, 4))
update!(element, "geometry", X)
bi = BasisInfo(Quad4)
element_info!(bi, element, (0.0,0.0), 0.0)

As a result, the cross product of the column vectors of the Jacobian is a normal direction and "size factor" is the norm of the normal vector:

t1 = bi.J[1,:]
t2 = bi.J[2,:]
n = cross(t1, t2)/norm(cross(t1,t2))

# output

3-element Array{Float64,1}:
 -0.7071067811865475
  0.0
  0.7071067811865475
detJ1 = norm(cross(n1,n2))
detJ2 = sqrt(det(bi.J*bi.J'))
detJ1 == detJ2 == bi.detJ == sqrt(2)/4

#output

true

For curves,
image

So is this the r'(t) in here?

Some links:

@ahojukka5
Copy link
Member

ahojukka5 commented Oct 15, 2018

Should we consider implementing also functions to return a single shape function, given its number? get_basis(xi, 1) and so on?

@coveralls
Copy link

Coverage Status

Coverage decreased (-41.08%) to 58.257% when pulling 3601116 on kc/tensors into 02da5c5 on master.

@ahojukka5 ahojukka5 merged commit 6aef1eb into master Nov 26, 2019
@ahojukka5 ahojukka5 deleted the kc/tensors branch November 26, 2019 07:47
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.

4 participants