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

Need gen support for restriction-only identity operators #1728

Open
zatkins-dev opened this issue Jan 15, 2025 · 15 comments · May be fixed by #1730
Open

Need gen support for restriction-only identity operators #1728

zatkins-dev opened this issue Jan 15, 2025 · 15 comments · May be fixed by #1730

Comments

@zatkins-dev
Copy link
Collaborator

zatkins-dev commented Jan 15, 2025

Trying to apply such an operator (effectively a restricted copy operation) results in the error:

libCEED/backends/cuda-gen/ceed-cuda-gen-operator-build.cpp:935 in CeedOperatorBuildKernel_Cuda_gen(): Backend does not implement restriction only identity operators

We use these operators for MPM.

Since these are such a specific type of operation, we could probably add support with a specialized kernel.

@zatkins-dev
Copy link
Collaborator Author

@jeremylt

@jeremylt
Copy link
Member

Can you point me to the line in Ratel? The Ratel test suite passed for me

@zatkins-dev
Copy link
Collaborator Author

zatkins-dev commented Jan 15, 2025

ratel/src/materials/ratel-mpm.c:3467

    // Apply composite operator
    RatelCallCeed(ratel, CeedOperatorApply(op_state, CEED_VECTOR_NONE, state_ceed, CEED_REQUEST_IMMEDIATE));
[0]PETSC ERROR: #1 RatelMPMAcceptState() at /home/zatkins/project/micromorph/ratel/src/materials/ratel-mpm.c:3467
[0]PETSC ERROR: #2 RatelTSPostEvaluate() at /home/zatkins/project/micromorph/ratel/src/internal/ratel-petsc-ops.c:594
[0]PETSC ERROR: #3 TSPostEvaluate() at /home/zatkins/project/micromorph/petsc/src/ts/interface/ts.c:3250
[0]PETSC ERROR: #4 TSSolve() at /home/zatkins/project/micromorph/petsc/src/ts/interface/ts.c:4085
[0]PETSC ERROR: #5 main() at /home/zatkins/project/micromorph/ratel/examples/ex02-quasistatic.c:71

@jeremylt
Copy link
Member

And is there a reason we can't veccopy?

@zatkins-dev
Copy link
Collaborator Author

You know, I thought there was, but now I'm not so sure. I'll try swapping over to copying the CeedVector

@zatkins-dev
Copy link
Collaborator Author

Okay, so the issue is that we need to use CeedVectorCopyStrided, as even with CEED_COPY_VALUES, CeedVectorCopy will get rid of the borrowed array and allocate a new owned array.

@jeremylt
Copy link
Member

huh, that shouldn't be happening

@zatkins-dev
Copy link
Collaborator Author

I figured as much -- that's how the logic is baked in for some reason. I also think there may be some weird host/device sync issues related to copying, but I can't say for sure what they are.

@jeremylt
Copy link
Member

jeremylt commented Jan 15, 2025

Ack, we're using CeedVectorSetArray internally which drops the old array. Probably we need a new method on CeedVectors that is vec->CopyArray(vec, mem_type, array). That method would need to be set for GPU backends, and if its not present, then we can fallback to memcpy in CeedVectorCopy().

I can whip it up tomorrow morning pretty quick unless you'd like to take a crack at it.

@zatkins-dev
Copy link
Collaborator Author

I probably won't get to it today or tomorrow, so go ahead! Thanks

@jedbrown
Copy link
Member

I'm not really following the need for vec->CopyArray. Where would that memory come from? Is there a reason not to wrap it in a CeedVector and then CeedVectorCopy instead of introducing a new way to use a raw pointer (without specifying its length, but tacitly promising that it's long enough)?

@jeremylt
Copy link
Member

That's exactly what we should do, but the internal logic on how to do that throws away the wrapped array when it should not

1 similar comment
@jeremylt
Copy link
Member

That's exactly what we should do, but the internal logic on how to do that throws away the wrapped array when it should not

@jedbrown
Copy link
Member

"throws away" something that is not array_owned?

@jeremylt
Copy link
Member

The user facing interface is correct - my impl is bad. I am fixing it

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

Successfully merging a pull request may close this issue.

3 participants