search

nbren12 / geostreams

Streaming data from fortran climate/weather models.
7 stars 3 forks source link

semi-generic redi_push #13

Closed jhamman closed 7 years ago

jhamman commented 7 years ago

partially closes #7

cc @nbren12

nbren12 commented 7 years ago

Overall, my preference would be to avoid using the preprocessor if possible and check the endianess using some of the solutions on this stackoverflow thread.

nbren12 commented 7 years ago

@jhamman Should we move this PR to a new branch like feature/fortran-polymorphism? That way I can push some changes.

nbren12 commented 7 years ago

I just tried the to compile and I am getting amongst other errors.

 CALL redis_push(redis, 'A-f4', f4)
                                    1
Error: There is no specific subroutine for the generic 'redis_push' at (1)

This is happening because of the rank mismatch between f4, and the declared dimension of arr in redis_push_f4(c, key, arr).

To make this work we would need to implement methods like redis_push_{datatype}_{ndims}. For example, if we want to implement this interface for real, float, and int arrays of up to 4 dimensions, it would require 12 different functions, which could ultimately be combined using an interface. I think this difficulty is why MPI asks user to specify manually specify a dtype and size when sending messages.

IMO at this early point, I think it is probably better to keep the code base small, and ask the user to pass the dtype and shape. I do think we should check the endianess in the code, but I am pretty sure __BYTE_ORDER__ is a gfortran specific macro, so we should do this check at runtime.

jhamman commented 7 years ago

If you can give me another day, I think I'm pretty close on working out these kinks. I have to run to a meeting but I think I've gotten most of this figured out at this point.

nbren12 commented 7 years ago

sure thing. take your time.

On Tue, Sep 26, 2017 at 12:51 PM, Joe Hamman notifications@github.com wrote:

If you can give me another day, I think I'm pretty close on working out these kinks. I have to run to a meeting but I think I've gotten most of this figured out at this point.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nbren12/geostreams/pull/13#issuecomment-332315288, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUokjcsUnLDL4Ca6x-m5e3YsqPC-l37ks5smVW_gaJpZM4PjNg5 .

jhamman commented 7 years ago

@nbren12 - ready for a review. Not 100% satisfied with the API like this but it seems to be working.

nbren12 commented 7 years ago

I am merging this into a feature branch