Rblp / Rblpapi

R package interfacing the Bloomberg API from https://www.bloomberglabs.com/api/
Other
167 stars 75 forks source link

Unable to install, "unknown architecture" error. #390

Closed attharmirza closed 1 month ago

attharmirza commented 2 months ago

I'm trying to install Rblpapi on a M1 Pro Macbook, running macOS 14.5, and getting the following console output:

Package which is only available in source form, and may need compilation of C/C++/Fortran: ‘Rblpapi’
Do you want to attempt to install these from sources? (Yes/no/cancel) Yes
installing the source package ‘Rblpapi’

trying URL 'https://lib.stat.cmu.edu/R/CRAN/src/contrib/Rblpapi_0.3.14.tar.gz'
Content type 'application/x-gzip' length 179467 bytes (175 KB)
==================================================
downloaded 175 KB

Unknown architecture: arm64. Exiting.

The downloaded source packages are in
    ‘/private/var/folders/89/9_4qkz810gl100py_vdpwv980000gq/T/RtmpfJfrhW/downloaded_packages’
Warning message:
In install.packages("Rblpapi") :
  installation of package ‘Rblpapi’ had non-zero exit status
attharmirza commented 2 months ago

Looks like it's failing CRAN checks as well https://cloud.r-project.org/web/checks/check_results_Rblpapi.html

eddelbuettel commented 2 months ago

There is AFAIK no M1/M2/M3 "arm64" binary for the blpapi library offered by Bloomberg (though there may be by now but I am unaware). So our setup still assumes that "macOS == x86_64".

attharmirza commented 2 months ago

I see, that makes sense. Thank you for clarifying! It seems like there is one now but it's still an experimental release. I'm assuming support won't be added until there's a stable release?

eddelbuettel commented 2 months ago

Yes, I looked as well, ever so briefly and only on my phone. We could try to help you help yourself and maybe see about extending the build that way. @johnlaing and I don't have such macOS machines.

johnlaing commented 2 months ago

I do indeed see the ARM libraries, however they are a newer version of the API. MacOS has always been "experimental" so that's not a roadblock for us. We are overdue for an update but I'll need to test Rblpapi with the new version before making that the default. If it works as-is then it will be straightforward to add support for arm64. Hopefully I can give that a test this weekend.

attharmirza commented 2 months ago

I appreciate you both looking into this so much, thank you! I am relatively inexperienced with C++ and R so I'm not sure how much you'll actually be able to help me help myself @eddelbuettel 😂, but if there's anything I can do to assist with testing and such please let me know.

klin333 commented 2 months ago

Hi @johnlaing, if and when Rblpapi switches to a new Bloomberg library version, will Windows users like myself have a way to change/configure Rblpapi ourselves to keep using the same Bloomberg library version? Very hesitant to upgrade something so foundational that’s not breaking for my own uses.

johnlaing commented 2 months ago

Good news: the package compiles on both windows and linux with the new headers and libraries. Bad news: there are many new warnings, most about deprecated functions. I haven't been able to test any actual functionality yet due to some quirks of my work environment at the moment.

@attharmirza If you'd like to take a shot at building/testing it yourself I think that would be a good approach. @klin333 This is a fair question. I need to think about what the right approach is (and likely @eddelbuettel has thoughts here) but I think I'm not ready to cross that bridge yet anyway.

eddelbuettel commented 2 months ago

@johnlaing We can noodle over the deprecation warnings in slack. I just got back home from traveling for a few days and I should be able to catch up. Supporting arm64 on macOS seems like a good thing worth doing.

@klin333 You always have old releases if you are particularly fixed on one. We aim to never break anything (unit tests help) so a new version should never be "worse".

eddelbuettel commented 2 months ago

@johnlaing Still running a little behind :roll_eyes: but check the 'labs' site have now three local copies blpapi at the 3.24.6.1. So maybe I should try to (ab)use the GitHub Actions facility and try to update the build for Linux and switch macOS to arm. What are your thoughts?

(And ... it has been a while. No longer quite remember why split the download into 'headers' (still per-arch) and 'library'. Ah well. Details :grin: ...)

johnlaing commented 2 months ago

@eddelbuettel that's not a bad start, but GH Actions only gets us so far. We'd need to run the full test suite with a live BBG connection to really get comfortable.

eddelbuettel commented 2 months ago

@johnlaing Oh yes for sure -- agree 100% that we do.

But the 'purely mechanical' issues of compiling and linking and, say, reasoning over 'deprecated' warnings from the compiler work without it.

johnlaing commented 2 months ago

Last time I updated the libraries I used an experimental branch both here and in blp: https://github.com/Rblp/Rblpapi/compare/experimental

eddelbuettel commented 2 months ago

Yes I (re-)found the one here earlier today too but had forgotten about the matching one at the blp repo. Thanks for the reminder!

eddelbuettel commented 2 months ago

I had a very first quick stab at the deprecation warnings and it ain't that bad. For the first file I looked, src/bdp.cpp the diff is pretty straightforward (include removing an obsolete Emacs config line (as EditorConfig is better -- so I should add a .editorconfig) and updating copyright year):

modified   src/bdp.cpp
@@ -1,9 +1,8 @@
-// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-
 //
 //  bdp.cpp -- "Bloomberg Data Point" query function for the BLP API
 //
 //  Copyright (C) 2013         Whit Armstrong
-//  Copyright (C) 2015 - 2016  Whit Armstrong and Dirk Eddelbuettel
+//  Copyright (C) 2015 - 2024  Whit Armstrong and Dirk Eddelbuettel
 //
 //  This file is part of Rblpapi
 //
@@ -43,6 +42,7 @@ using BloombergLP::blpapi::Event;
 using BloombergLP::blpapi::Element;
 using BloombergLP::blpapi::Message;
 using BloombergLP::blpapi::MessageIterator;
+using BloombergLP::blpapi::Name;

 void getBDPResult(Event& event, Rcpp::List& res, const std::vector<std::string>& securities, const std::vector<std::string>& colnames, const std::vector<RblpapiT>& rtypes, bool verbose) {
     MessageIterator msgIter(event);
@@ -55,17 +55,17 @@ void getBDPResult(Event& event, Rcpp::List& res, const std::vector<std::string>&
     if (std::strcmp(response.name().string(),"ReferenceDataResponse")) {
         throw std::logic_error("Not a valid ReferenceDataResponse.");
     }
-    Element securityData = response.getElement("securityData");
+    Element securityData = response.getElement(Name{"securityData"});

     for (size_t i = 0; i < securityData.numValues(); ++i) {
         Element this_security = securityData.getValueAsElement(i);
-        size_t row_index = this_security.getElement("sequenceNumber").getValueAsInt32();
+        size_t row_index = this_security.getElement(Name{"sequenceNumber"}).getValueAsInt32();

         // check that the seqNum matches the order of the securities vector (it's a grave error to screw this up)
-        if(securities[row_index].compare(this_security.getElementAsString("security"))!=0) {
+        if(securities[row_index].compare(this_security.getElementAsString(Name{"security"}))!=0) {
             throw std::logic_error("mismatched Security sequence, please report a bug.");
         }
-        Element fieldData = this_security.getElement("fieldData");
+        Element fieldData = this_security.getElement(Name{"fieldData"});
         for(size_t j = 0; j < fieldData.numElements(); ++j) {
             Element e = fieldData.getElement(j);
             auto col_iter = std::find(colnames.begin(), colnames.end(), e.name().string());

So we may just have to stick a bunch of Name{...} instantiations in and might be able to call it a day.

eddelbuettel commented 2 months ago

That was in fact the only type of warning, and I have it covered now across all relevant source files. And because the Name class existed 'forever' the change also works with our current (outdated) library and header so I can prepare a PR for that independently of the library change.

eddelbuettel commented 2 months ago

The new branch feature/release_3.24.6.1 builds Linux and macOS arm64 successfully in continuous integration via GitHub Actions, and I also (with an additional manual tweak to src/Makevars.win that is just pointing to that branch too) build on win-builder, see the logs and binary here.

I will make another minor cleanup PR for the main branch which we should merge first.

eddelbuettel commented 1 month ago

@attharmirza We have a new arm64 binary using the current Bloomberg API SDK for you to test. If you go to the macbuilder results page and download from the link named results.tar.bz2 you get an archive with build artifacts including the new package big-sur-arm64/bin/4.4/Rblpapi_0.3.14.1.tgz which you should be able to install and test.

Could you let us know if that works for you on arm64 macOS?

eddelbuettel commented 1 month ago

Please see the main branch as well as the binaries at r-universe -- you should be able to run on arm64 macOS now as it is supported.