-
-
Notifications
You must be signed in to change notification settings - Fork 547
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
Adds gradient_squared
method to FiniteVolume
#4540
base: develop
Are you sure you want to change the base?
Conversation
gradient_squared
method to
FiniteVolume`gradient_squared
method to FiniteVolume
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4540 +/- ##
===========================================
- Coverage 99.27% 99.21% -0.06%
===========================================
Files 300 300
Lines 22795 22809 +14
===========================================
+ Hits 22630 22631 +1
- Misses 165 178 +13 ☔ View full report in Codecov by Sentry. |
@medha-14 This PR should also add tests and use the gradient squared function where appropriate |
54edbdb
to
f8a506e
Compare
I have written a test for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some changes, mostly to improve coverages, but getting there!
def gradient_squared(self, symbol, discretised_symbol, boundary_conditions): | ||
""" | ||
Computes the square of the gradient of a symbol. | ||
|
||
Parameters | ||
---------- | ||
symbol : :class:`pybamm.Symbol` | ||
The symbol for which to compute the gradient squared. | ||
discretised_symbol : :class:`pybamm.Vector` | ||
The discretised variable for which to compute the gradient squared. | ||
boundary_conditions : dict | ||
Boundary conditions for the symbol. | ||
|
||
Returns | ||
------- | ||
float | ||
The gradient squared of the symbol. | ||
""" | ||
domain = symbol.domain | ||
gradient_matrix = self.gradient_matrix(domain, symbol.domains) | ||
|
||
# Compute gradient squared: (∇u)^2 = u^T (L^T L) u | ||
gradient_squared_matrix = gradient_matrix.T @ gradient_matrix | ||
gradient_squared_result = ( | ||
discretised_symbol.T @ gradient_squared_matrix @ discretised_symbol | ||
) | ||
|
||
return gradient_squared_result.item() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, boundary_conditions
is mentioned in the function's signature, but it doesn't look like it's used in the logic of the function anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback. I've updated the gradient_squared
function to correctly handle boundary conditions, and I've adjusted the tests accordingly to verify these changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @medha-14!
I noticed that in the |
@agriyakhetarpal, I would like to give this a go and explore the implementation further. I’d be grateful for any additional insights or recommendations you might have as I move forward. |
Description
Fixes #2979
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run
(or$ nox -s pre-commit
) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all
(or$ nox -s tests
)$ python run-tests.py --doctest
(or$ nox -s doctests
)You can run integration tests, unit tests, and doctests together at once, using
$ python run-tests.py --quick
(or$ nox -s quick
).Further checks: