safer nu #911
safer nu #911
Conversation
📝 WalkthroughWalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Ohm
participant Layout
User->>Ohm: Call spatial_hyperresistive_()
Ohm->>Layout: Calculate laplacian(J(component), index)
Ohm->>Ohm: Compute hyperresistive term using min_density
Ohm-->>User: Return computed value
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/core/numerics/ohm/ohm.hpp (3)
363-364: LGTM with a minor suggestion.The separation of
lvlCoeffcalculation improves readability. The introduction ofmin_densityaligns with the PR objective of enhancing stability for low magnetic field strengths.Consider using a more descriptive name for
min_density, such asmin_density_threshold, to better convey its purpose in preventing division by zero or near-zero values.
372-373: Approved: Formula update aligns with PR objectives.The updated formula
(b / (nOnE + min_density) + 1)effectively addresses the issue of the expression becoming too small when the magnetic field strength approaches zero. This change enhances the stability of the calculations, particularly around reconnection X points.Consider pre-computing the inverse of
(nOnE + min_density)to potentially improve performance:- return -nu_ * (b / (nOnE + min_density) + 1) * lvlCoeff + auto inv_denominator = 1.0 / (nOnE + min_density); + return -nu_ * (b * inv_denominator + 1) * lvlCoeff * layout_->laplacian(J(component), index);This optimization could be beneficial if this calculation is performed frequently in performance-critical sections.
Line range hint
363-389: LGTM: Well-structured implementation with improved stability.The
spatial_hyperresistive_method is well-implemented, using a lambda function to reduce code duplication and correctly handling different components with their respective projections. The changes successfully address the PR objectives by improving the stability of calculations when the magnetic field strength approaches zero.Consider adding a brief comment explaining the purpose of the
min_densityconstant and how it contributes to the stability of the calculations. This would enhance the maintainability of the code for future developers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/core/numerics/ohm/ohm.hpp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/core/numerics/ohm/ohm.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.
|
Ok so here is a result for this new PR. The upper panel uses constant hyper-resistivity We see that it is too dissipative. Typically if the term is non-negligible on L0, dissipation will be too large on L2 (because the grid scale decreases relative to the dissipative scale imposed by Second panel uses from master. Third panel uses the proposed formula with In contrast to the master version, dissipation clearly operates around the X point despite Last panel is the same as above with larger nu
here a zoom version
|
replaces
which becomes too small when$B\to 0$ , typically around reconnection X points.
There, increasing$\nu_0$ is not solving the problem, it only increases dissipation in regions where B is non-zero and cannot prevent the dissipation to go to zero with B.
The proposed formula is:
which tends to:
with
when$B\to 0$
and
when$n\to 0$
For reference, here is what we get un run055 with constant hyper-resistivity$\nu=0.002$ :