-
Notifications
You must be signed in to change notification settings - Fork 8
add reordering to/from DIR_C with openMP #101
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. To check my understanding, the two new indexing cases are simply stating "If you want the Cartesian coordinates of the Cartesian layout you already have them"?
Yes, exactly |
src/common.f90
Outdated
integer, intent(out) :: dir_from, dir_to | ||
integer, intent(in) :: rdr_dir | ||
|
||
select case (rdr_dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about using findloc
intrinsic in Fortran instead of the select case
block? Haven't really experimented with this before but looks like we can use it to obtain dir_from
and dir_to
indices easily with a single query using the rdr_map
array.
https://gcc.gnu.org/onlinedocs/gcc-12.3.0/gfortran/FINDLOC.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that's what I was wondering too. Performance wise I am not sure what's the best. The other option would be two lut for from_dir
and to_dir
of size 31 (with lots of zeros).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it can be bad for performance, this one is called very often inside the reorder loop. Maybe its better to explore different options later when we're investigating the performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, that call wasn't needed so I removed it. It should be better now performance wise, regardless of the implementation
I believe it is ready now |
add reordering to/from DIR_C with openMP
closes #100