-
Notifications
You must be signed in to change notification settings - Fork 18
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
New hierarchy for Mapping classes #429
base: devel
Are you sure you want to change the base?
Conversation
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.
Great job @Sworzzy!
You have erronously included some macOS hidden files in the PR
I've deleted all the .DS_Store files and modified the .gitignore suscessfully. |
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.
Hey @Sworzzy, good effort! I am posting a partial review.
Co-authored-by: Yaman Güçlü <[email protected]>
Co-authored-by: Yaman Güçlü <[email protected]>
_pdim = 2 | ||
_expressions = {'x': 'a * (x1 + eps / (2*pi) * sin(2*pi*x1) * sin(2*pi*x2))', | ||
'y': 'b * (x2 + eps / (2*pi) * sin(2*pi*x1) * sin(2*pi*x2))'} | ||
|
||
mapping = CollelaMapping2D('M', a=1, b=1, eps=.2) |
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.
the CollelaMapping2D
from sympde
has k1
and k2
as parameters. This line is probably wrong.
This PR is to link with Sympde PR#168, where the new hierarchy is showed.
The interface is now simpler. An example can be in the files changed of this PR, in psydac/api/postprocessing.py, in the _get_mesh function l. 2286 :
There is no callable mapping anymore. So the type of the mapping now is either : spline, analytic or None. ANd you may do directly evaluations on the object without getting a callable argument or so.
This new hierarchy maintains "domain undefined mapping", that you may get from the Domain.from_file method.
Now, in the tests, the real mappings (the "defined" or "callable" version), can be obtained from domain_h.mappings (returns a list of mappings, either spline or analytic) : see psydac.api.tests.test_api_feec_2d.py
An improvement could be done in this mapping encapsulation. Which class should have the "defined" mapping, to have the most user-friendly tests. For example in psydac.api.tests.test_api_feec_2d.py, you must set the mapping attribute, of the derham object :