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

Warning indicates probable bug in nf90_open handling of cache_preemption #146

Closed
edhartnett opened this issue Mar 1, 2019 · 2 comments · Fixed by #149
Closed

Warning indicates probable bug in nf90_open handling of cache_preemption #146

edhartnett opened this issue Mar 1, 2019 · 2 comments · Fixed by #149

Comments

@edhartnett
Copy link
Contributor

We have this warning:

netcdf4_func.f90:84:27:

           preemption_out = cache_preemption
                           1
Warning: Possible change of value in conversion from REAL(4) to INTEGER(4) at (1) [-Wconversion]
netcdf4_func.f90:39:27:

           preemption_out = cache_preemption
                           1
Warning: Possible change of value in conversion from REAL(4) to INTEGER(4) at (1) [-Wconversion]
netcdf_text_variables.f90:60:93:

      localMap   (:numDims  ) = (/ 1, (product(localCount(:counter)), counter = 1, numDims - 1) /)
                                                                                             1
Warning: DO loop at (1) will be executed zero times [-Wzerotrip]
netcdf_attributes.f90:221:17:

     valuesA(1) = values
                 1
Warning: Possible change of value in conversion from INTEGER(8) to INTEGER(4) at (1) [-Wconversion]
netcdf4_file.f90:49:25:

         preemption_out = cache_preemption
                         1
Warning: Possible change of value in conversion from REAL(4) to INTEGER(4) at (1) [-Wconversion]
netcdf4_func.f90:783:75:

When I look at the code I see that we are simply casting the real to the integer preemption. However I believe this should be multiplied by 100 first.

I will check this out next time I take a look at netcdf-fortran, but I've just put up a PR and I will wait til that gets merged...

@edhartnett edhartnett changed the title Warning indicates probable bug in Warning indicates probable bug in nf90_open handling of cache_preemption Mar 1, 2019
@edhartnett
Copy link
Contributor Author

I should note that this bug would not be easy to notice, since an incorrect cache preemption value would only effect performance. I will see if I can add a test for this condition as well...

@edhartnett
Copy link
Contributor Author

edhartnett commented Mar 2, 2019

As suspected, the overloaded nf_create_par()/nc_open_par() functions use cache_preemption incorrectly.

Due to extra cfortran.h madness which related to passing floats as parameters, for the fortran APIs the cache preemptions (for file and var) are converted from a real between 0 and 1 to an int between 0 and 100, by simply multiplying by 100 and taking an INT() of the result.

This is build into the F77 api, and the F90 is essentially a fancy wrapper around the F77 API, so the same applies there.

However, this was not done when these overloaded functions were added. As a result, whenever the users have attempted to use this parameter, their cache preemption is getting set to 0, because the code is just assigning the float, which is < 1, to the int, which will always get 0.

This is not something anyone would particularly notice, so even if people are doing this, they would not know.

The fix is to properly multiply the preemtion by 100 first, and that will get the users' intended preemptions to the C library.

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 a pull request may close this issue.

1 participant