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

Support for operator boundary conditions #22

Closed
thomasmelvin opened this issue Apr 10, 2017 · 6 comments
Closed

Support for operator boundary conditions #22

thomasmelvin opened this issue Apr 10, 2017 · 6 comments
Assignees

Comments

@thomasmelvin
Copy link
Collaborator

Boundary conditions are applied to fields through calls to enforce_bc_kernel that are either explicitly written by the algorithm generator or automatically applied after calls to matrix_vector_kernel for W1 and W2 function spaces.

It has been decided to directly include the boundary conditions inside of operators through the use of a enforce_operator_bc_kernel that modifies a operator type. LFRic ticket https://code.metoffice.gov.uk/trac/lfric/ticket/999 implements this new kernel and this issue is to replace the psykalite call with psyclone support.

Additionally with the boundary conditions placed in the appropriate operators there will no longer be any need for automatic generation of enforce_bc calls after matrix_vector calls and so this can be removed.

@arporter arporter self-assigned this Apr 11, 2017
arporter added a commit that referenced this issue Apr 11, 2017
@arporter
Copy link
Member

Created new operator_bcs branch for this work.
As a first step I've modified the tests that check for automatic inclusion of enforce_bc_kern after any occurrence of matrix_vector.

@arporter
Copy link
Member

enforce_operator_bc_kernel_type has the operator access as GH_INC rather than GH_WRITE. Tom explains:

"This is the first time an operator has been gh_inc, it is because the boundary conditions are enforced as:

Operator(I,j,k) = operator(I,j,k)*bc(I,j,k)

Where bc is either 0 or 1, so we need to have intent(inout)
there is still no write contention, each kernel call will write to discontinuous data so from a psyclone point of view I guess it can be treated the same as gh_write (in terms of looping etc) so maybe we could change it to gh_write?"

I think we'll need to leave it as GH_INC for the time being so as to ensure that the corresponding argument is given INTENT(inout). The only penalty for this is that the corresponding loop will have to be coloured when parallelising with OpenMP. However, we may want to do that anyway to minimise data movement.

The 'proper' solution is to introduce a GH_READWRITE access. I'll migrate the corresponding SRS ticket (831) onto github.

@arporter
Copy link
Member

@thomasmelvin : does the new enforce_operator_bc_kernel only work with LMA operators?

@thomasmelvin
Copy link
Collaborator Author

Yes
We could write one for cma's but since all cma's are currently made from lma's I dont think this is needed

@arporter
Copy link
Member

Thanks Tom. I've updated the documentation to describe the new enforce_operator_bc_kernel and have tweaked eg4 to show the use of such a kernel.

rupertford added a commit that referenced this issue May 16, 2017
Merge request for issue #22 - apply BCs to operators
@arporter
Copy link
Member

#26 has been merged to master. Closing issue.

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

No branches or pull requests

2 participants