-
Notifications
You must be signed in to change notification settings - Fork 84
Update Laplace
class and add unit tests
#645
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
Conversation
c3bd828
to
cc7840f
Compare
I would not remove Laplace, maybe add a fix laplacian but I would keep the Laplace (as an equation). In principle we could add Burgers, Advection, ... |
The FixedLaplacian class allows to do the same thing as Laplace by setting value to 0. The class was renamed for consistency with other classes within the same file. Many of the mentioned equations are inside the problem definitions in the problem zoo. Do we want to move them all in the equation factory and import them for the problem zoo classes definitions? |
Will leave Laplace class for back compatibility. It will simply inherit from FixedLaplacian with value set to 0. |
51782a3
to
2de792c
Compare
Hi @dario-coscia, @FilippoOlivo,
Note that these changes have no impact on the user, as nothing "importable" has been modified in its behaviour. |
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.
Hi @GiovanniCanali! Thanks for the PR, to me it looks great and more structured as so. I made one minor comment for the tutorial file.
It would be great to also add the mathematical description of the PDE in Poisson
, Laplace
, Advection
, ect. Maybe we can do this in the doc?
2de792c
to
f231ea7
Compare
f231ea7
to
779102c
Compare
Hi @dario-coscia, |
Description
This PR fixes #644
Checklist