-
Notifications
You must be signed in to change notification settings - Fork 102
Hessians for embedded elements #1257
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1257 +/- ##
==========================================
- Coverage 94.19% 94.19% -0.01%
==========================================
Files 40 40
Lines 6662 6679 +17
==========================================
+ Hits 6275 6291 +16
- Misses 387 388 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lijas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Can you also add the hessian equation (for embeded elements) to the docs, similar to how we do it here: https://ferrite-fem.github.io/Ferrite.jl/stable/topics/FEValues/#Identity-mapping
And add a reference to where it is taken from (or perhaps show the derivation :) )
| J += otimes_helper(x[j], dMdξ) | ||
|
|
||
| # J += x[j] ⊗ dMdξ | ||
| # H += x[j] ⊗ d2Mdξ2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where did the computation of the hessian go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, I only added it in above. This code then might not be tested atm. Also, what is the difference between calculate_mapping(gip::ScalarInterpolation) and calculate_mapping(geo_mapping::GeometryMapping). They seem quite similar.
| @test shape_hessian(cv_ref2, qp, 1)[1, 1, 1] ≈ shape_hessian(cv_ref2n, qp, 1)[1, 1, 1] | ||
| @test shape_hessian(cv_ref2, qp, 1)[1, 2, 1] ≈ shape_hessian(cv_ref2n, qp, 1)[1, 2, 1] | ||
| @test shape_hessian(cv_ref2, qp, 1)[2, 1, 1] ≈ shape_hessian(cv_ref2n, qp, 1)[2, 1, 1] | ||
| @test shape_hessian(cv_ref2, qp, 1)[2, 2, 1] ≈ shape_hessian(cv_ref2n, qp, 1)[2, 2, 1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can add tests for the function_hessian also, as we do here:
coords_nl = [x + rand(x) * 0.01 for x in coords] # add some displacement to nodes
reinit!(cv, coords_nl)
_ue_nl = [u_funk(coords_nl[i], V, G, H) for i in 1:n_basefunc_base]
ue_nl = reinterpret(Float64, _ue_nl)
for i in 1:getnquadpoints(cv)
xqp = spatial_coordinate(cv, i, coords_nl)
Hqp, Gqp, Vqp = Tensors.hessian(x -> function_value_from_physical_coord(func_interpol, coords_nl, x, ue_nl), xqp, :all)
@test function_value(cv, i, ue_nl) ≈ Vqp
@test function_gradient(cv, i, ue_nl) ≈ Gqp
if update_hessians
@test Ferrite.function_hessian(cv, i, ue_nl) ≈ Hqp
end
end
Can you add a similar test in this @testset for hessians? (I am not sure if function_value_from_physical_coord works with embedded elements)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be more difficult than i first thought. For the embedded case, the function_value_from_physical_coord would need some signed distance search to find the parametric coord on the surface
| @inline function dothelper(A::SMatrix{vdim, sdim}, B::SArray{Tuple{sdim, rdim, rdim}}) where {vdim, rdim, sdim} | ||
| return SArray{Tuple{vdim, rdim, rdim}}( | ||
| (dothelper(A[i, :], B)[j, k] for i in 1:vdim, j in 1:rdim, k in 1:rdim) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this type of construction of SArrays is fine, but I dont know how efficiently the compiler can optimize it, maybe someone else can review this part :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were right, there are a lot of calls in there.
This implementation takes 24 ns on my system, I found a better version which only takes about 2 ns
|
Thanks @lijas for the review. I will adress the rest of your points when I have more time :) |
|
I compared the results from function_hessian from the hessian computed with AD: The function value and gradient test passes, but the hessian does not. So I suspect that there is some error in you hessian computation. For this test to work, I needed to change this function to |
|
@lijas Thanks, okay then I have to take a second look. It's a bit tricky to see where the error could be, its either in the computation of the Hessian (of the geometry) or in the Edit: I think the error should be in |
This should be ready for a first review.
This implements two things:
I have used the same approach as was used for jacobians, i.e. overloading functions like
dothelper, etc. This might not be the prettiest and could possibly be implemented a bit better. Maybe someone has a better idea, on how to implement some of the new helper methods.Linking also for reference : Ferrite-FEM/Tensors.jl#188