Skip to content

Fix for broadcast_file for NAG compiler based on broadcast_string#88

Open
DJDavies2 wants to merge 7 commits into
ecmwf:developfrom
DJDavies2:feature/81
Open

Fix for broadcast_file for NAG compiler based on broadcast_string#88
DJDavies2 wants to merge 7 commits into
ecmwf:developfrom
DJDavies2:feature/81

Conversation

@DJDavies2
Copy link
Copy Markdown
Contributor

Description

Fix broadcast_file for NAG compiler by copying the solution for broadcast_string.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

@DJDavies2
Copy link
Copy Markdown
Contributor Author

I don't know why the character handling is done differently in broadcast_string to broadcast_path, it seems to have been done a long time ago. It ought to be possible to write a function to do this. I guess that is what c_str is supposed to be? Maybe there is a problem with it.

@DJDavies2
Copy link
Copy Markdown
Contributor Author

Looking at c_str, it doesn't do the same thing as the chunk of code in broadcast_string (and now broadcast_file). c_str just appends a c_null_char, I'm not sure why one couldn't just do that directly? The code chunk in broadcast_string converts a fortran string into an array of single character strings with a c_null_char appended, which is a very different thing.

@DJDavies2
Copy link
Copy Markdown
Contributor Author

One calls fckit__mpi__broadcast_string, the other fckit__mpi__broadcast_file. However these have the same declaration for the string arguments (an assumed size array of len=1 characters of kind c_char). The code chunk matches that whereas c_str doesn't.

@DJDavies2
Copy link
Copy Markdown
Contributor Author

Fix for #81

@wdeconinck
Copy link
Copy Markdown
Member

Thanks for this fix. I see the problem now, and I have tried another fix in #89 which should hopefully also work and avoids the copies altogether, and should be more performant.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants