Skip to content

Comments

ECC-2074: Fortran: get_message, new_from_message without memcpy#279

Merged
shahramn merged 7 commits intoecmwf:developfrom
plesskem:feature/get_message
Apr 24, 2025
Merged

ECC-2074: Fortran: get_message, new_from_message without memcpy#279
shahramn merged 7 commits intoecmwf:developfrom
plesskem:feature/get_message

Conversation

@plesskem
Copy link
Contributor

Proposed extension of the fortran grib api:
2 methods which are already included in the C interface and used to directly access the coded message in main memory
these methods are for example practical for sending/receiving coded grib messages with MPI

  • grib_get_message
    returns a fortran pointer to the coded message (character) of a grib_handle
  • grib_new_from_message_no_copy
    creates a new grib_handle with a data buffer pointing to a provided coded message (character or integer)

@FussyDuck
Copy link

FussyDuck commented Dec 18, 2024

CLA assistant check
All committers have signed the CLA.

@shahramn shahramn self-assigned this Apr 22, 2025
@shahramn shahramn added the approved-for-ci Approved to run CI on ECMWF machines label Apr 22, 2025
@shahramn
Copy link
Collaborator

Compilation fails. Can you please take a look
e.g.,

eccodes/fortran/grib_fortran.cc:905:12: error: no matching function for call to 'grib_get_message'
  iret = grib_get_message(h,&message,mess_len);
src/eccodes_prototypes.h:539:5: note: candidate function not viable:
no known conversion from 'void **' to 'const void **' for 2nd argument
 int grib_get_message(const grib_handle* h, const void** msg, size_t* size);

@github-actions github-actions bot removed the approved-for-ci Approved to run CI on ECMWF machines label Apr 23, 2025
@shahramn shahramn added the approved-for-ci Approved to run CI on ECMWF machines label Apr 23, 2025
@shahramn
Copy link
Collaborator

Still failing:

eccodes/fortran/grib_fortran.cc: In function ‘int grib_f_get_message_(int*, const void**, size_t*)’:
eccodes/fortran/grib_fortran.cc:905:31: error: invalid conversion from ‘void**’ to ‘const void**’ [-fpermissive]
      iret = grib_get_message(h,&message,mess_len);
                                ^~~~~~~~
In file included from src/eccodes/grib_api_internal.h:193,
                  from eccodes/fortran/grib_fortran.cc:11:
eccodes/src/eccodes/grib_api.h:571:57: note: initializing argument 2 of ‘int grib_get_message(const grib_handle*, const void**, size_t*)’

@plesskem
Copy link
Contributor Author

Sorry for the compilation failure! I am correcting and testing it locally. If it is successful, I'll notify you.

@github-actions github-actions bot removed the approved-for-ci Approved to run CI on ECMWF machines label Apr 23, 2025
@plesskem
Copy link
Contributor Author

My tests locally were successful and I noticed that I had only support for size_t message length and therefore added support for int as well.
Fingers crossed that your tests run through now. :)

@shahramn shahramn added the approved-for-ci Approved to run CI on ECMWF machines label Apr 24, 2025
@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2025

Codecov Report

Attention: Patch coverage is 82.85714% with 12 lines in your changes missing coverage. Please review.

Project coverage is 87.70%. Comparing base (848f4b1) to head (d8a55c5).
Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
examples/F90/grib_get_message.f90 84.31% 8 Missing ⚠️
fortran/grib_fortran.cc 78.94% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #279      +/-   ##
===========================================
- Coverage    87.71%   87.70%   -0.01%     
===========================================
  Files          843      844       +1     
  Lines        62339    62409      +70     
  Branches     11126    11133       +7     
===========================================
+ Hits         54680    54738      +58     
- Misses        7659     7671      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shahramn
Copy link
Collaborator

Thank you. Could you please add a test too? similar to:
examples/F90/grib_copy_message.f90
so when we run ctest, the code you have added gets exercised and we know it is functioning as expected

@github-actions github-actions bot removed the approved-for-ci Approved to run CI on ECMWF machines label Apr 24, 2025
@shahramn shahramn added the approved-for-ci Approved to run CI on ECMWF machines label Apr 24, 2025
@shahramn shahramn changed the title get_message, new_from_message without memcpy ECC-2074: Fortran: get_message, new_from_message without memcpy Apr 24, 2025
@shahramn shahramn merged commit 2215f61 into ecmwf:develop Apr 24, 2025
206 of 208 checks passed
@shahramn
Copy link
Collaborator

Thank you for your contribution 🥇

@plesskem plesskem deleted the feature/get_message branch April 24, 2025 17:23
@shahramn
Copy link
Collaborator

This has now been released as part of ecCodes v2.42.0

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

Labels

approved-for-ci Approved to run CI on ECMWF machines contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants