feat: electrostatics#3698
Conversation
lihanyu97
requested review from
CusiniM,
castelletto1,
cssherman,
dkachuma,
jhuang2601,
paveltomin and
rrsettgast
as code owners
lihanyu97
requested review from
Guotong-Ren,
MelReyCG,
corbett5,
matteofrigo5 and
wrtobin
as code owners
paveltomin
removed request for
CusiniM,
Guotong-Ren,
matteofrigo5 and
paveltomin
frankfeifan
left a comment
frankfeifan
left a comment
There was a problem hiding this comment.
I think it's in a good shape. Will need to fix the styling issues and better to add some new baseline tests.
There was a problem hiding this comment.
Better to rename the xmls to something easier to understand the features of the example, e.g., 1D with interface, etc.
Also, I'd suggest adding some small problems to integrated tests.
There was a problem hiding this comment.
Same. I'd suggesting adding examples to integrated tests.
| virtual void postInputInitialization() override; | ||
|
|
||
| string m_fieldName; | ||
| TimeIntegrationOption m_timeIntegrationOption; |
There was a problem hiding this comment.
Is m_timeIntegrationOption needed?
| }); | ||
| } | ||
|
|
||
| void Electrostatics::applyButlerVolmerCurrent(DofManager const& dofManager, DomainPartition& domain, |
There was a problem hiding this comment.
Better to make a kernel for it, but not urgent I think.
There was a problem hiding this comment.
I mimic the pattern in solid mechanics solver. It's a bit strange it has all the volume integration within kernels but the interface integration is implemented directly inside the solver like this.
lihanyu97
added
the
ci: run code coverage
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3698 +/- ##
===========================================
- Coverage 58.87% 58.75% -0.12%
===========================================
Files 1353 1366 +13
Lines 112953 113534 +581
===========================================
+ Hits 66504 66711 +207
- Misses 46449 46823 +374 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| set(CMAKE_CXX_COMPILER "/usr/tce/packages/gcc/gcc-12.1.1-magic/bin/g++" CACHE PATH "") | ||
|
|
||
| set(CMAKE_Fortran_COMPILER "/usr/tce/packages/gcc/gcc-12.1.1-magic/bin/gfortran" CACHE PATH "") |
There was a problem hiding this comment.
Can you please pull out any changes to the host-config files as a separate (small) PR? We want to be able to easily track such changes, and they are unrelated to this physics PR.
Also, none of the other configs seem to contain Fortran info. Why is this change necessary? Any changes should likely be as consistent as possible across all configs.
There was a problem hiding this comment.
I don't think it's necessary anymore. Will revert it back.
4 participants
Initial implementation of electrostatics system with Butler-Volmer reaction computed with jumps on the split surface