-
Notifications
You must be signed in to change notification settings - Fork 19
sometimes capture by reference #511
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
base: main
Are you sure you want to change the base?
Conversation
…singularity-eos into jmm/sometimes-capture-by-reference
I'm not sure we want to support fortran device binding unless there's a dedicated person willing to invest in that. But it sounds like a scalar fortran interface for the host would be useful, yes? |
Yes I think that's right. I'm not sure how many LOC it would be but I think it would be a useful addition. |
|
@Yannicked I am returning to this as we haven't had any movement towards a scalar fortran API. Does this branch resolve your performance issues? |
PR Summary
As discussed in #508 @Yannicked reports problems with the vector API for small vectors. This was partially resolved by #509 . In general, it would be better to use the cell based API in this case. However, specifically for CPU, this can be resolved by capturing by reference instead of by value in the
portableFor. This MR adds this optionally. It is off by default and comes with significant warning labels.This is marked WIP as I'm not sure we want to pursue this. It is, as far as I can tell, safe here. But does fundamentally change the semantics of these functions in some cases, which I consider a risk as far as debugging and maintainability.
An alternative approach would be to expose scalar fortran functions. That would be a lot more boiler plate, but potentially more maintainable. Curious to hear folks thoughts. @dholladay00 @jhp-lanl .
Note that getting scalar fortran functions to bind onto device is apparently a challenge, as C/fortran interop is a fundamentally host-side thing. Going this path would at least let us punt on that until that machinery is potentially more mature.
PR Checklist
make formatcommand after configuring withcmake.If preparing for a new release, in addition please check the following:
when='@main'dependencies are updated to the release version in the package.py