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

Multi-output library function #888

Closed
jowr opened this issue Dec 3, 2015 · 7 comments
Closed

Multi-output library function #888

jowr opened this issue Dec 3, 2015 · 7 comments
Milestone

Comments

@jowr
Copy link
Member

jowr commented Dec 3, 2015

Let us agree on a new function for the CoolProp interface as a follow up regarding the discussion on the mailing list.
My new suggestion is:

// A function with the most common outputs, `value1` and `value2` are arrays of the length `length`
// and so are the output properties `T`, `p`, `rho`, `h`, `s`
EXPORT_CODE void CONVENTION AbstractState_update_common(const long handle, const long input_pair, const double* value1, const double* value2, const long length, double* T, double* p, double* rho, double* h, double* s)

// A function with selected outputs, `value1` and `value2` are arrays of the length `length`
// and so are the output pointers 
// `out0`, `out1`, `out2`, `out3`, `out4`, `out5`, `out6`, `out7`, `out8` and `out9`.
// The outputs are filled according to the content of the `outputs` array of the length
// `output_len` that contains long keys from `CoolProp::parameters`
EXPORT_CODE void CONVENTION AbstractState_update_output(const long handle, const long input_pair, const double* value1, const double* value2, const long length, double* out0, double* out1, double* out2, double* out3, double* out4, double* out5, double* out6, double* out7, double* out8, double* out9, const long* outputs, const long output_len)
@EdoMacchi
Copy link

For me this should work. I did a test with a similar function (with pointers but not with arrays) and it works. The problem is that the performances of this new function (for update+computation of rho and Cp) are worse than the one of the standard functions (three different calls). Is it possible this is due to the use of pointers? Anyway probably the overall performances will improve when using arrays.

@EdoMacchi
Copy link

Ok, tested with the arrays. This is much faster (probably even faster than the C++ low-level interface). I will post the C and Matlab code shortly.

@EdoMacchi
Copy link

What I did is for two properties, but it can be easily generalized. Tested a bit the performances: in general slightly better than C++ low-level interface for HEOS (16 s vs 17 s) and TTSE (0.15 s vs 0.2 s) and better than C++ for BICUBIC (0.31 vs 0.75).

In CoolPropLib.cpp I added the function:

EXPORT_CODE void CONVENTION AbstractState_updateCompProp(const long handle, const long input_pair, const double* value1, const double* value2, const long param1, const long param2, double* out1, double* out2, const long length)
{
        shared_ptr<CoolProp::AbstractState> &AS = handle_manager.get(handle);

        for (int i=0;i<length;i++){
        AS->update(static_cast<CoolProp::input_pairs>(input_pair), *(value1+i), *(value2+i));

        *(out1+i) = AS->keyed_output(static_cast<CoolProp::parameters>(param1));
        *(out2+i) = AS->keyed_output(static_cast<CoolProp::parameters>(param2));
        }

}

Then to use it in Matlab (starting from the last script posted in the mailing list):

Pptr = libpointer('doublePtr',P);
Tptr = libpointer('doublePtr',T);
out1ptr = libpointer('doublePtr',zeros(N,1));
out2ptr = libpointer('doublePtr',zeros(N,1));

calllib('cool','AbstractState_updateCompProp',handle,c_PT,Pptr,Tptr,c_rhomass,c_cpmass,out1ptr,out2ptr,N);
rho(:,1)=get(out1ptr,'Value');
Cp(:,1)=get(out2ptr,'Value');

Thank you for your suggestion!
Best regards,

Edo

@ibell
Copy link
Contributor

ibell commented Dec 3, 2015

That is already looking good.

We obviously need to add error checking and things like that, but given the performance, seems well worth it.

My proposal would be along the lines of @jowr :

// A function with the most common outputs, `value1` and `value2` are arrays of the length `length`
// and so are the output properties `T`, `p`, `rho`, `h`, `s`
EXPORT_CODE void CONVENTION AbstractState_updateoutput_common(const long handle, const long input_pair, const double* value1, const double* value2, const long length, double* T, double* p, double* rhomolar, double* hmolar, double* smolar, char *errbuffer, long buffer_length)

Thinking about it, it could be useful to be able to pass an error message to say the handle is invalid, etc.
And then we have a generalized one, with up to 5 outputs:

// A function with the most common outputs, `in1` and `in2` are arrays of the length `length`
// outputs is a 5-element array with the integer keys for the desired outputs.  Alternatively, it could be a delimited string, which would allow for returning derivatives.
EXPORT_CODE void CONVENTION AbstractState_updateoutput_custom(const long handle, const long input_pair, const double* in1, const double* in2, const long length, const long *outputs, double* out1, double* out2, double* out3, double* out4, double* out5, char *errbuffer, long buffer_length)

@ibell
Copy link
Contributor

ibell commented Dec 4, 2015

@EdoMacchi Are you willing to take this to the point of developing code that would be ready to be merged into CoolProp? I don't think it would take too long. It would be best if you could write the function, some docs, and open a pull request.

@ibell ibell added this to the v5.2 milestone Dec 4, 2015
@EdoMacchi
Copy link

I would like to help but since I don't know so much the code I think that it would take some time to get both functions working (this morning I tried with the simple one and I got some unexpected problems :( ). In addition right now I don't have a lot of free time... Maybe I can give you a hand with the documentation...

Update - found the problem: for some strange reason it didn't like some long functions names (AbstractState_updateoutput_common, AbstractState_update_output_common and other two shorter names that I tried). With AbstractState_updtOutComm it works.

Please find hereinafter the function with the common outputs (I am not sure if error management is ok, I copied it from the std functions). Some more work is probably needed for the other function since it has to manage the case where there is no output for some pointers.

In CoolPropLib.h:

    EXPORT_CODE void CONVENTION AbstractState_updtOutComm(const long handle, const long input_pair, const double* value1, const double* value2, const long length, double* T, double* p, double* rhomolar, double* hmolar, double* smolar, long *errcode, char *message_buffer, const long buffer_length);
    /**
     * @brief Update the state of the AbstractState and get an output value five common outputs (temperature, pressure, molar density, molar enthalpy and molar entropy)
     * @brief from the AbstractState using pointers as inputs and output to allow array computation.
     * @param handle The integer handle for the state class stored in memory
     * @param input_pair The integer value for the input pair obtained from XXXXXXXXXXXXXXXX
     * @param value1 The pointer to the array of the first input parameters
     * @param value2 The pointer to the array of the second input parameters
     * @param length The number of elements stored in the arrays
     * @param T The pointer to the array of temperature
     * @param p The pointer to the array of pressure
     * @param rhomolar The pointer to the array of molar density
     * @param hmolar The pointer to the array of molar enthalpy
     * @param smolar The pointer to the array of molar entropy
     * @param errcode The errorcode that is returned (0 = no error, !0 = error)
     * @param message_buffer A buffer for the error code
     * @param buffer_length The length of the buffer for the error code
     * @return 
     */

In CoolPropLib.cpp:

EXPORT_CODE void CONVENTION AbstractState_updtOutComm(const long handle, const long input_pair, const double* value1, const double* value2, const long length, double* T, double* p, double* rhomolar, double* hmolar, double* smolar, long *errcode, char *message_buffer, const long buffer_length)
{
    *errcode = 0;
    try{
        shared_ptr<CoolProp::AbstractState> &AS = handle_manager.get(handle);

        for (int i=0;i<length;i++){
        AS->update(static_cast<CoolProp::input_pairs>(input_pair), *(value1+i), *(value2+i));

        *(T+i) = AS->T();
        *(p+i) = AS->p();
        *(rhomolar+i) = AS->rhomolar();
        *(hmolar+i) = AS->hmolar();
        *(smolar+i) = AS->smolar();
        };
    }
    catch(CoolProp::HandleError &e){
        std::string errmsg = std::string("HandleError: ") + e.what();
        if (errmsg.size() < static_cast<std::size_t>(buffer_length)){
            *errcode = 1;
            strcpy(message_buffer, errmsg.c_str());
        }
        else{
            *errcode = 2;
        }
    }
    catch(CoolProp::CoolPropBaseError &e){
        std::string errmsg = std::string("Error: ") + e.what();
        if (errmsg.size() < static_cast<std::size_t>(buffer_length)){
            *errcode = 1;
            strcpy(message_buffer, errmsg.c_str());
        }
        else{
            *errcode = 2;
        }
    }
    catch(...){
        *errcode = 3;
    }
}

Best,

Edo

@ibell ibell modified the milestones: v5.1.2, v5.2 Dec 13, 2015
ibell added a commit that referenced this issue Dec 13, 2015
@ibell
Copy link
Contributor

ibell commented Dec 14, 2015

Anyone have any recommendations for eliminating potential problems like not providing a large enough buffer? For these, there isn't much we can do to protect ourselves against. I vote we close this issue, and we can add others if there is a clear need.

@ibell ibell closed this as completed Dec 14, 2015
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

No branches or pull requests

3 participants