Skip to content

Conversation

@jowezarek
Copy link
Contributor

@jowezarek jowezarek commented Sep 26, 2023

Adds the kernel eval_field_3d_once to psydac.core.field_evaluation_kernels.py and makes use of it in psydac.fem.tensor.py's method eval_field.

The purpose of this kernel is to accelerate the evaluation of a single 3d field at a single point. This is needed repeatedly, for example, when creating a Poincaré plot tracing a single fieldline.

To Do:

  • Check for possibly faster implementation (@yguclu)
  • Add 1D & 2D kernel (@yguclu)

@jowezarek jowezarek marked this pull request as ready for review September 26, 2023 17:55
@jowezarek jowezarek marked this pull request as draft September 28, 2023 11:23
@jowezarek
Copy link
Contributor Author

The current use of template does not work @yguclu:

@template(name='T1', types=['float[:,:,:]', 'complex[:,:,:]'])
@template(name='T2', types=['float[:]', 'complex[:]'])

I get the error message

ERROR at annotation (semantic) stage
pyccel:
 |error [semantic]: field_evaluation_kernels.py [28,16]| Variable has already been defined with type 'float64'. Type 'complex128' in assignment is incompatible (res=c3 * local_bases_0[i] * local_bases_1[j] * local_bases_2[k])

Locally I will switch back to the float-only case for now. Do you know how to properly make sure that dtype complex can also be handled?

@jowezarek jowezarek requested a review from yguclu October 18, 2023 11:09
@yguclu
Copy link
Member

yguclu commented Oct 19, 2023

Related to issue #188

@yguclu
Copy link
Member

yguclu commented Mar 6, 2024

The current use of template does not work @yguclu:

@template(name='T1', types=['float[:,:,:]', 'complex[:,:,:]'])
@template(name='T2', types=['float[:]', 'complex[:]'])

I get the error message

ERROR at annotation (semantic) stage
pyccel:
 |error [semantic]: field_evaluation_kernels.py [28,16]| Variable has already been defined with type 'float64'. Type 'complex128' in assignment is incompatible (res=c3 * local_bases_0[i] * local_bases_1[j] * local_bases_2[k])

Locally I will switch back to the float-only case for now. Do you know how to properly make sure that dtype complex can also be handled?

The templates should work now, provided that you upgrade Pyccel to version >= 1.11.2

@vcarlier
Copy link
Contributor

vcarlier commented Mar 7, 2024

The templates should work now, provided that you upgrade Pyccel to version >= 1.11.2

I just updated my Pyccel and the error is still there. Could you try on your machine?

@vcarlier
Copy link
Contributor

vcarlier commented Mar 7, 2024

@yguclu
I cannot reproduce the error in the tests on my laptop (even when running pytest in parallel), do you have any idea why?

@yguclu
Copy link
Member

yguclu commented Mar 8, 2024

@yguclu I cannot reproduce the error in the tests on my laptop (even when running pytest in parallel), do you have any idea why?

You did not forget to run the script psydac_accelerate.py, did you?

@yguclu
Copy link
Member

yguclu commented Mar 8, 2024

@yguclu I cannot reproduce the error in the tests on my laptop (even when running pytest in parallel), do you have any idea why?

You did not forget to run the script psydac_accelerate.py, did you?

I see the problem now. This branch has not been updated with devel, so it is still using the old version of Pyccel.

@yguclu
Copy link
Member

yguclu commented Mar 8, 2024

@jowezarek Is there a reason for allowing the basis functions to be complex valued?

Instead of this code

@template(name='T1', types=['float[:,:,:]', 'complex[:,:,:]'])
@template(name='T2', types=['float[:]', 'complex[:]'])
def eval_field_3d_once(local_coeffs: 'T1', 
                       local_bases_0: 'T2', local_bases_1: 'T2', local_bases_2: 'T2'):

we can just write

@template(name='T', types=[float, complex])
def eval_field_3d_once(local_coeffs: 'T[:,:,:]', 
                       local_bases_0: 'float[:]', local_bases_1: 'float[:]', local_bases_2: 'float[:]'):

@jowezarek
Copy link
Contributor Author

@jowezarek Is there a reason for allowing the basis functions to be complex valued?

Instead of this code

@template(name='T1', types=['float[:,:,:]', 'complex[:,:,:]'])
@template(name='T2', types=['float[:]', 'complex[:]'])
def eval_field_3d_once(local_coeffs: 'T1', 
                       local_bases_0: 'T2', local_bases_1: 'T2', local_bases_2: 'T2'):

we can just write

@template(name='T', types=[float, complex])
def eval_field_3d_once(local_coeffs: 'T[:,:,:]', 
                       local_bases_0: 'float[:]', local_bases_1: 'float[:]', local_bases_2: 'float[:]'):

@yguclu, is it possible that your proposed solution was not possible prior to your template-update PR?
Either way, that is exactly what I wanted in the first place. I never intended to define a template for complex coefficients and real basis function values / real coefficients and complex basis function values.

@yguclu
Copy link
Member

yguclu commented Mar 13, 2024

@jowezarek Is there a reason for allowing the basis functions to be complex valued?
Instead of this code

@template(name='T1', types=['float[:,:,:]', 'complex[:,:,:]'])
@template(name='T2', types=['float[:]', 'complex[:]'])
def eval_field_3d_once(local_coeffs: 'T1', 
                       local_bases_0: 'T2', local_bases_1: 'T2', local_bases_2: 'T2'):

we can just write

@template(name='T', types=[float, complex])
def eval_field_3d_once(local_coeffs: 'T[:,:,:]', 
                       local_bases_0: 'float[:]', local_bases_1: 'float[:]', local_bases_2: 'float[:]'):

@yguclu, is it possible that your proposed solution was not possible prior to your template-update PR? Either way, that is exactly what I wanted in the first place. I never intended to define a template for complex coefficients and real basis function values / real coefficients and complex basis function values.

My PR related to templates makes this function easier to read, but even without it it was possible to write this:

@template(name='T', types=['float[:,:,:]', 'complex[:,:,:]'])
def eval_field_3d_once(local_coeffs: 'T', 
                       local_bases_0: 'float[:]', local_bases_1: 'float[:]', local_bases_2: 'float[:]'):

@vcarlier
Copy link
Contributor

@yguclu everything seems to be fixed now

@yguclu yguclu added the urgent PR should be merged ASAP label Nov 13, 2024
@yguclu yguclu self-assigned this Aug 27, 2025
@yguclu yguclu added the performance Make the code run faster label Aug 27, 2025
@yguclu yguclu changed the title Add single 3d field evaluation kernel Add evaluation kernels for single FemField Aug 27, 2025
@codacy-production
Copy link

codacy-production bot commented Aug 27, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.07% 67.50%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (776ac29) 31894 19552 61.30%
Head commit (2595d9c) 64004 (+32110) 39190 (+19638) 61.23% (-0.07%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#335) 200 135 67.50%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@yguclu
Copy link
Member

yguclu commented Aug 27, 2025

The Documentation / build_docs job is failing with the error

Extension error (sphinx.ext.autosummary): Handler <function process_generate_options ...> for event 'builder-inited' threw an exception (exception: no module named psydac.api)

I am really surprised given that we have made no change to the documentation infrastructure. I have asked Copilot and it says that this happens because Psydac cannot be imported. So it suggests to either install Psydac, or add its source directory to the PYTHONPATH. I am not convinced, but I will try the latter...

@jowezarek: What do you think?

@yguclu
Copy link
Member

yguclu commented Aug 28, 2025

The Documentation / build_docs job is failing with the error

Extension error (sphinx.ext.autosummary): Handler <function process_generate_options ...> for event 'builder-inited' threw an exception (exception: no module named psydac.api)

I am really surprised given that we have made no change to the documentation infrastructure. I have asked Copilot and it says that this happens because Psydac cannot be imported. So it suggests to either install Psydac, or add its source directory to the PYTHONPATH. I am not convinced, but I will try the latter...

@jowezarek: What do you think?

I have decided to install Psydac as this only takes 2 minutes. Things appear to work now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Make the code run faster urgent PR should be merged ASAP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants