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

Simplify and speedup shape_gradient_and_value for embedded #947

Merged
merged 3 commits into from
May 25, 2024

Conversation

KnutAM
Copy link
Member

@KnutAM KnutAM commented May 24, 2024

Noticed this when reviewing #938

ip = Lagrange{RefQuadrilateral,1}()^3; ξ = rand(Vec{2});
@btime Ferrite.shape_gradient_and_value($ip, $ξ, 1);
# PR:     15.030 ns (0 allocations: 0 bytes)
# master: 26.459 ns (1 allocation: 64 bytes)

Copy link

codecov bot commented May 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.84%. Comparing base (c956e70) to head (6e2b4a8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #947      +/-   ##
==========================================
- Coverage   93.84%   93.84%   -0.01%     
==========================================
  Files          36       36              
  Lines        5317     5310       -7     
==========================================
- Hits         4990     4983       -7     
  Misses        327      327              

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

@termi-official
Copy link
Member

Since this is already done I am okay with merging this, but shouldn't we rather try to finalize Ferrite-FEM/Tensors.jl#188 (comment) ?

@fredrikekre
Copy link
Member

Need to re-resolve the doc manifest to include ForwardDiff as a dependency.

@KnutAM
Copy link
Member Author

KnutAM commented May 25, 2024

but shouldn't we rather try to Tensors.jl#188

I would love to, but need quite some input on Ferrite-FEM/Tensors.jl#211 before doing a managable implementation of Ferrite-FEM/Tensors.jl#188 (and that will probably be after Ferrite 1.0)

I simply did this to check my understanding of the code, since @lijas re-uses this code in #938. So perhaps not clear from the PR, but since I then had this code the main merit is to simplify for others to see what is happening here, instead of relying on Tensor internals. (On the other hand, this code was probably beneficial for generalizing to the 3rd order case, but will look at that now if a similiar simplification can be suggested in 938).

@KnutAM KnutAM mentioned this pull request May 25, 2024
3 tasks
@KnutAM KnutAM merged commit 41cfff5 into master May 25, 2024
10 checks passed
@KnutAM KnutAM deleted the kam/embedded_opt branch May 25, 2024 12: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.

3 participants