Fixes capacitance calculation #35
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the capacitance and inductance matrix calculations for open problems by implementing Paul's formula 5.21. The key changes include refactoring the matrix calculation logic and adding comprehensive test coverage for the new implementation.
- Refactors capacitance matrix calculation to separate generalized and standard matrix computations
- Implements Paul's formula 5.21 for converting generalized capacitance matrices to standard capacitance matrices
- Updates existing tests with more accurate tolerance values and adds new validation tests
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Driver.h | Updates method signatures to separate generalized and standard capacitance matrix calculations |
| src/Driver.cpp | Implements Paul's formula 5.21 and refactors matrix calculation logic |
| test/DriverTest.cpp | Adds comprehensive test coverage and updates tolerance values for improved accuracy |
| .gitmodules | Changes submodule URLs from SSH to HTTPS format |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| mfem::DenseMatrix Driver::getCFromGeneralizedC( | ||
| const mfem::DenseMatrix& gC, | ||
| const Model::Openness& opennness) |
There was a problem hiding this comment.
Parameter name 'opennness' contains a spelling error. It should be 'openness' (one 'n').
| res.Invert(); | ||
| res *= MU0_NATURAL * EPSILON0_NATURAL; | ||
| return res; |
There was a problem hiding this comment.
The inductance matrix calculation is missing the multiplication by MU0_NATURAL * EPSILON0_NATURAL that was present in the original code. This will result in incorrect inductance values.
| // Comparison with Clayton Paul's book: | ||
| // Analysis of multiconductor transmision lines. 2007. |
There was a problem hiding this comment.
Word 'transmision' is misspelled. It should be 'transmission'.
| // Comparison with Clayton Paul's book: | |
| // Analysis of multiconductor transmision lines. 2007. | |
| // Analysis of multiconductor transmission lines. 2007. |
| // Comparison with Clayton Paul's book: | ||
| // Analysis of multiconductor transmision lines. 2007. |
There was a problem hiding this comment.
Word 'transmision' is misspelled. It should be 'transmission'.
| // Comparison with Clayton Paul's book: | |
| // Analysis of multiconductor transmision lines. 2007. | |
| // Analysis of multiconductor transmission lines. 2007. |
Fixes capacitance and inductance matrices calcualtion for open problems. It was being calculated incorrectly. Now implementes Paul's formula 5.21