Skip to content
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

Open
wants to merge 228 commits into
base: devel
Choose a base branch
from
Open

New hierarchy for Mapping classes #429

wants to merge 228 commits into from

Conversation

Sworzzy
Copy link

@Sworzzy Sworzzy commented Sep 4, 2024

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 :

if use_spline_mapping:
        domain_h = discretize(domain, filename=filename, comm=MPI.COMM_WORLD) 
        derham_h = discretize(derham, domain_h, multiplicity = [mult, mult])
        
        # TO FIX : the way domain.from_file and discretize_domain runs make that only domain_h.domain.interior has an actual 
         SplineMapping. The rest are BaseMapping.
        # The trick is to now when to set exactly the BaseMapping to the SplineMapping. We could try in discretize_derham, but 
        see if it doesn't generate any issues for other tests.

        mappings=list(domain_h.mappings.values())
        
        if(len(mappings)>1):
            raise TypeError("we are not doing multipatch here")
        
        mapping = mappings[0] # mapping is SplineMapping now
        derham_h.mapping=mapping

Copy link
Member

@yguclu yguclu left a 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

.DS_Store Outdated Show resolved Hide resolved
.github/.DS_Store Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
doc/.DS_Store Outdated Show resolved Hide resolved
docs/.DS_Store Outdated Show resolved Hide resolved
psydac/.DS_Store Outdated Show resolved Hide resolved
psydac/api/.DS_Store Outdated Show resolved Hide resolved
psydac/cad/.DS_Store Outdated Show resolved Hide resolved
psydac/feec/.DS_Store Outdated Show resolved Hide resolved
psydac/mapping/.DS_Store Outdated Show resolved Hide resolved
@Sworzzy
Copy link
Author

Sworzzy commented Sep 6, 2024

I've deleted all the .DS_Store files and modified the .gitignore suscessfully.

@Sworzzy Sworzzy requested a review from yguclu September 9, 2024 08:34
Copy link
Member

@yguclu yguclu left a 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.

.gitignore Outdated Show resolved Hide resolved
.github/workflows/documentation.yml Outdated Show resolved Hide resolved
examples/poisson_2d_mapping.py Outdated Show resolved Hide resolved
mesh/generate_pipe.py Outdated Show resolved Hide resolved
mesh/multipatch/create_magnet.py Outdated Show resolved Hide resolved
psydac/cad/geometry.py Outdated Show resolved Hide resolved
psydac/cad/geometry.py Outdated Show resolved Hide resolved
psydac/cad/geometry.py Outdated Show resolved Hide resolved
psydac/cad/multipatch.py Outdated Show resolved Hide resolved
psydac/cad/tests/pipe.h5 Outdated Show resolved Hide resolved
@yguclu yguclu changed the title New Mapping hierarchy New hierarchy for Mapping classes Sep 25, 2024
@yguclu yguclu requested a review from a team September 25, 2024 12:40
_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)
Copy link
Collaborator

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.

@yguclu yguclu requested a review from a team October 16, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants