Minor changes in transient_heat_equation.jl and change in notation of test functions#1305
Minor changes in transient_heat_equation.jl and change in notation of test functions#1305Avina-cK wants to merge 8 commits intoFerrite-FEM:masterfrom
Conversation
In transient_heat_equation.jl: - Assembly function of K,M,f merged to one. - Added reference to fe_into
Avina-cK
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1305 +/- ##
=======================================
Coverage 94.40% 94.40%
=======================================
Files 40 40
Lines 6846 6846
=======================================
Hits 6463 6463
Misses 383 383 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| The solution to the equation above is usually calculated from the corresponding weak | ||
| form. By multiplying the equation with an arbitrary *test function* $\delta u$, integrating | ||
| form. By multiplying the equation with an arbitrary *test function* $\varphi$, integrating |
There was a problem hiding this comment.
I definitely see the point made by the contributor here @KnutAM . Since we explain it here from the Bubnov-Galerkin perspective I see that the notation
I am just not sure about the choice of
There was a problem hiding this comment.
δu can indeed be a bit confusing
Yes, but I also think that for most users it will make things easier if we are always consistent that test function for x is δx, even if mathematically the variation and test functions are different. Alternatively, we should have another consistent scheme (but that would require something like φᵤ or uₜ to have a good scheme for coupled problems). Using the subscripts quickly becomes confusing though, since we typically want to reserve these for indices (e.g. old value in time-dependent problems)
Specifically using φ is not good, since we used that for the shape functions in the intro to FEM (we use N in other places).
In the code, we are currently mixing using the notation for the test function and function itself with the test shape function and shape function. (I'm no fan of that but others are :D)
My preferred notation
For a field x,
- Function
x(obviously) ($x \approx N_i a_i$ ) - Test function
δx(but highlight in the intro to FEM that we use this even though we argue from the "Bubnov-Galerkin perspective") ($\delta x \approx \delta N_i c_i$ ) - Use
Nin code forshape_valuefor the function interpolation - Use
δNin code forshape_valuefor the test function interpolation - In code, add letter
Nx/δNxin coupled problems (and$N^x$ /$\delta N^x$ in math notation) - For assembly, we can subscript these, e.g.
∇δNᵢ⋅D⋅∇Nⱼor∇δNuᵢ⋅D⋅∇Npⱼ
Of course, we can exchange N -> φ, but easier to type N and consistent with the internal implementation in FunctionValues.
\varphi and \phi also renders a bit differently depending, sometimes \varphi looks like \phi, so we shouldn't use both for different things.
There was a problem hiding this comment.
And noting @fredrikekre's comment here: #1304 (comment) - I guess that would be ok if we note that we (ab)use the notation δx for the test function even though it is not necessarily a variation.
And regarding the transient heat problem specifically, I think this can be derived from a potential so it can actually be considered a variation, but this would fail for other cases such as partially saturated porous media.
I guess we could introduce a different notation instead of δ, e.g. 𝒯 (\scrT), but I'm guessing this would be more confusing for most users even if we are consistent?
| # The semidiscrete weak form is given by | ||
| # ```math | ||
| # \int_{\Omega}v \frac{\partial u}{\partial t} \ \mathrm{d}\Omega + \int_{\Omega} k \nabla v \cdot \nabla u \ \mathrm{d}\Omega = \int_{\Omega} f v \ \mathrm{d}\Omega, | ||
| # \int_{\Omega}\delta u \frac{\partial u}{\partial t} \ \mathrm{d}\Omega + \int_{\Omega} k \nabla \delta u \cdot \nabla u \ \mathrm{d}\Omega = \int_{\Omega} f \delta u \ \mathrm{d}\Omega, |
There was a problem hiding this comment.
Maybe I misunderstood you, but didn't you wanted to change these to be consistent with the heat example? Before changing these, let us wait for a quick feedback on https://github.com/Ferrite-FEM/Ferrite.jl/pull/1305/changes#r2934003577 .
There was a problem hiding this comment.
😅right. I must have missed that part. Once we get feedback on which symbol would be better, I'll update the lot and fix the two broken reference links.
Fixed broken links and changed test functions (in heat-equation.jl and transient-heat-equation.jl) to δu.
|
All (except for one) examples' code sections are now using the following notation:
|
termi-official
left a comment
termi-official
left a comment
There was a problem hiding this comment.
Some drive-by comments on stuff I saw found when going over the proposed changes. No need to take action yet.
There was a problem hiding this comment.
This tutorial is not fully translated to the new syntax (see the pressure blocks).
There was a problem hiding this comment.
😅 My mistake, I switched it to review way too soon. I'm a bit new to Git and contributing; thanks for your patience.
There was a problem hiding this comment.
Absolutely not problem! :) Take your time. And no worries, we all understand that the first steps to open source contributions have a bit of a barrier due all to the nitty-gritty details like familiarizing one-self with git, documentation conventions, ... .
In the code of ns_vs_diffeq.jl: * v - Nvᵢ / Nvⱼ * ϕ - δNvᵢ/ δNvⱼ * ψ - δNpᵢ/ δNpⱼ * p - Npᵢ / Npⱼ Added a section describing notation in fe_intro.md.
|
All (except linearshell) tutorials have now been updated. I'm not sure I know how to update the linearshell one, so I left it as is. It is partially already using the 'N' notation and I think it is okay as is. A small section has been added in fe_intro describing the new notation. |
3 participants
In all changed files: Change in notation of test functions: δu, v -> ϕ
In transient_heat_equation.jl: