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

Add value- and char-array support to co_reduce functions. #332

Merged
merged 2 commits into from Feb 9, 2017

Conversation

vehre
Copy link
Collaborator

@vehre vehre commented Feb 8, 2017

Add support for recude-functions with value parameters.
Add support for char-arrays as arguments to reduce-functions.
Rename co_reduce_1 to internal_co_reduce to get a better naming.
Create mpi-datatype to transport the char-array length.
Improved cleanup of co_broadcast. Fixed co_reduce_string testcase.

Fixes #172.

    Add support for recude-functions with value parameters.
    Add support for char-arrays as arguments to reduce-functions.
    Rename co_reduce_1 to internal_co_reduce to get a better naming.
    Create mpi-datatype to transport the char-array length.
    Improved cleanup of co_broadcast. Fixed co_reduce_string testcase.

    Fixes #172.
@vehre
Copy link
Collaborator Author

vehre commented Feb 8, 2017

@zbeekman: On your comments on the previous pull request:

  • I left the comment in the CMakeLists.txt intentionally, because not only the commented phrase should be deleted, but also the testcases should be moved to the regular unit test directory once the patch is approved.
  • Its of course possible to use my editor to use space instead of tabs, but I don't see why :-), because there is no common style in the code and jumping over a tab while coding is far more easier than over 8 spaces.
  • I had trouble to amend the commit to fix the copyrightCLA and resolved this by producing a diff, throwing the whole commits away, applying the patch to a fresh master and commiting that. So no, I don't know how to amend a commit. And I don't care after working for more than 12 hours today.
  • Fixing style sometimes happens just because I had some debugging in there. So when deleting the debuging again a style change may occur.

You use my limited time for fixing bugs and adding features or for fixing style. Your choice! :-)

@codecov-io
Copy link

codecov-io commented Feb 9, 2017

Codecov Report

Merging #332 into master will increase coverage by 0.48%.

@@            Coverage Diff             @@
##           master     #332      +/-   ##
==========================================
+ Coverage   45.73%   46.22%   +0.48%     
==========================================
  Files           3        3              
  Lines        1019     1045      +26     
  Branches      193      201       +8     
==========================================
+ Hits          466      483      +17     
- Misses        476      483       +7     
- Partials       77       79       +2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f652a7b...09a679c. Read the comment docs.

@zbeekman
Copy link
Collaborator

zbeekman commented Feb 9, 2017

@zbeekman: On your comments on the previous pull request:

I left the comment in the CMakeLists.txt intentionally, because not only the commented phrase should be deleted, but also the test cases should be moved to the regular unit test directory once the patch is approved.

OK, you could probably have included it with the patch, but that's fine

Its of course possible to use my editor to use space instead of tabs, but I don't see why :-), because there is no common style in the code and jumping over a tab while coding is far more easier than over 8 spaces.

Obviously you don't use Emacs ;-p

I had trouble to amend the commit to fix the copyrightCLA and resolved this by producing a diff, throwing the whole commits away, applying the patch to a fresh master and commiting that. So no, I don't know how to amend a commit. And I don't care after working for more than 12 hours today.

That's perfectly acceptable... your approach is fine

Fixing style sometimes happens just because I had some debugging in there. So when deleting the debuging again a style change may occur.
You use my limited time for fixing bugs and adding features or for fixing style. Your choice! :-)

well I'm happy to clean up after you, at least until I get a better handle on the gfortran internals etc.... :-)

@zbeekman
Copy link
Collaborator

zbeekman commented Feb 9, 2017

LGTM

Approved with PullApprove

@zbeekman zbeekman merged commit 793813d into master Feb 9, 2017
@zbeekman zbeekman deleted the vehre/co_reduce_calltype_fix_172 branch February 9, 2017 19:36
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.

None yet

3 participants