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

create function to populate cell_in #251

Merged
merged 17 commits into from
Nov 9, 2023
Merged

Conversation

mlee03
Copy link

@mlee03 mlee03 commented Oct 19, 2023

This PR creates a new function called get_CellStruct. This function populates the component arrays of cell_in.

Copy link
Contributor

@ngs333 ngs333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comment.

@@ -62,7 +62,6 @@ void set_rotate_poly_true(void){
void error_handler(char *str)
error handler: will print out error message and then abort
***********************************************************/
#pragma acc routine seq
void error_handler(const char *msg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this function be cleaned up?
Also, it seems that there is an "exit(1)" that can be executed but without printing anything. Is this true?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't changed the exit(1) statement.
I think this is as clean as we can get this function right now.
We need to look up what kind of error handling OpenACC has and I was going to leave that til the end.

@mlee03
Copy link
Author

mlee03 commented Nov 7, 2023

I honestly have no idea what's happening, but I recently ran gpu_dev for benchmarking with respect to the CPU version and I'm seeing a lot more differences than I remember seeing several weeks ago (implying have I been doing something wrong this entire time?). The differences are in xgrid_area with absolute max difference of 0.0197258, and mean difference of -3.06172e-05 with stdev of 0.00179223.

The other differences are seen withtile1_distance with absolute max difference of 5.77101e-11, and mean difference of -5.30129e-15 with stdv of 8.7227e-13

Are we good with these differences?

@ngs333
Copy link
Contributor

ngs333 commented Nov 7, 2023

@mlee03

I honestly have no idea what's happening, but I recently ran gpu_dev for benchmarking with respect to the CPU version and I'm seeing a lot more differences than I remember seeing several weeks ago (implying have I been doing something wrong this entire time?). The differences are in xgrid_area with absolute max difference of 0.0197258, and mean difference of -3.06172e-05 with stdev of 0.00179223.

The other differences are seen withtile1_distance with absolute max difference of 5.77101e-11, and mean difference of -5.30129e-15 with stdv of 8.7227e-13

Are we good with these differences?

It depends. Max diff of 0.0197258 for an xgrid areas doesseem high; but if you know that the percent differences are sufficiently small, then its its OK. Note that cell areas can be ~10^7 meters square is SOME grids cells in some grids, so if 0.0197258 meter square difference occurs for a large cell, then that is not really so bad. What is the highest pct difference in the same test case?

@mlee03
Copy link
Author

mlee03 commented Nov 8, 2023

The percentage difference for xgrid_area seems to be small, but seeing large percentage difference (up to 100%) for tile1_distance but these numbers are somewhere in the 10^-15

@ngs333
Copy link
Contributor

ngs333 commented Nov 8, 2023

The percentage difference for xgrid_area seems to be small, but seeing large percentage difference (up to 100%) for tile1_distance but these numbers are somewhere in the 10^-15

This kind of error (tiny field values, large pct error) in floating point output is acceptable in CPU vs GPU comparisons.

@ngs333 ngs333 merged commit d5ee340 into NOAA-GFDL:gpu_dev Nov 9, 2023
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants