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

[LFRic] New builtin to initialise operators with pseudo-random data #2001

Closed
arporter opened this issue Jan 6, 2023 · 6 comments
Closed
Assignees
Labels
LFRic Issue relates to the LFRic domain question

Comments

@arporter
Copy link
Member

arporter commented Jan 6, 2023

The adjoint test-harness generation requires that operators be initialised with pseudo-random numbers. We already have setval_random for fields, this needs extending to operators.

@arporter
Copy link
Member Author

arporter commented Jan 6, 2023

As mentioned in #1864 I've rather brutally hacked my way to a solution. The major things that I've done that I'm not sure about are:

  1. Extending builtins to accept operator arguments (currently they accept only scalars and fields);
  2. Extending builtins to permit them to operate on the 'domain' (currently they must only operate on dofs);
  3. Allowing kernels that operate on the 'domain' to have operator arguments (currently only scalars and fields are permitted).

It's the last one that I'm particularly uneasy about. Ideally I'd change the validation to only permit this combination for builtin kernels but at the point the validation is performed we don't know that we have a builtin kernel (because the same code is used for both user-supplied and builtin kernels).

The alternative is to have the new builtin generate the correct looping structure to visit all elements of the local_stencil array and call random_number separately for each. The local_stencil array is of rank 3 with dimensions (ndf_to, ndf_from, ncell3d). The advantage of this is that it would limit the changes to just point 1. above.

@arporter
Copy link
Member Author

arporter commented Jan 6, 2023

If a 'user' (really a developer) was to implement a new builtin that modified a field but operated on the 'domain' then we almost certainly wouldn't do the right thing with the halos. My gut feeling is to try and implement a nested loop for the new builtin rather than have it operate on the domain. What do @TeranIvy and @rupertford think?

@arporter
Copy link
Member Author

arporter commented Jan 8, 2023

A much simpler solution (which I used some time ago) is to have a 'user-supplied' kernel that does the job. This has the big advantage that we don't have to change any code. The disadvantage is that it adds a bit more complexity to the build system for a mini-app that uses it (unless we make it a standard LFRic kernel??). I'm currently favouring this solution as the whole 'domain' kernel concept was originally supposed to just be a workaround for i-first kernels that contain their own OMP parallelisation.

@TeranIvy
Copy link
Collaborator

TeranIvy commented Jan 10, 2023

We have an issue for built-ins for operators, #1099.

Regarding the user-supplied kernel that would do the job, I am wondering whether a kernel that operates on a DoF would be appropriate here. The issue for that is #1351.

@arporter
Copy link
Member Author

Thanks very much for the pointer @TeranIvy. In this case and in the e.g. in #1099 the kernel needs to operate on CELL_COLUMNS I think because the operator is declared as (ndf_to, ndf_from, ncell_3d). On discussion with @rupertford, we also concluded that builtins are intended for common linear-algebra type operations whereas this is really a special case. Am happy to discuss further though.

@arporter
Copy link
Member Author

Am closing this for now as the immediate requirement is satisfied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LFRic Issue relates to the LFRic domain question
Projects
None yet
Development

No branches or pull requests

2 participants