Compile against FairMQ 1.3.6 by mkrzewic · Pull Request #1458 · AliceO2Group/AliceO2

mkrzewic · 2018-11-07T11:53:47Z

This is the bare minimum to make it chooch. There will be another PR that will integrate the new memory_resource stuff that got merged in FairMQ (thus removing most of the current O2 implementation).

sawenzel · 2018-11-07T13:13:10Z

I think the first step would be to get a compiling o2-dev-fairroot. Can we bump the versions there first?

sawenzel · 2018-11-07T14:53:19Z

@mkrzewic : Can you please fix the clang-format errors?

dberzano · 2018-11-08T09:07:09Z

Hi @mkrzewic, so thanks to alisw/alidist#1378 the deprecation warnings are gone in o2-dev-fairroot. We still have this error as you can see from the details:

/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_o2checkcode_o2/0/Utilities/O2MessageMonitor/src/O2MessageMonitor.cxx: In member function 'virtual void O2MessageMonitor::Run()':
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_o2checkcode_o2/0/Utilities/O2MessageMonitor/src/O2MessageMonitor.cxx:61:90: error: invalid conversion from 'const FairMQTransportFactory*' to 'FairMQTransportFactory*' [-fpermissive]
   auto dataResource = o2::memory_resource::getTransportAllocator(subChannels[0].Transport());
                                                                  ~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_o2checkcode_o2/0/Utilities/O2Device/include/O2Device/Utilities.h:30:0,
                 from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_o2checkcode_o2/0/Utilities/O2Device/include/O2Device/O2Device.h:33,
                 from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_o2checkcode_o2/0/Utilities/O2MessageMonitor/include/O2MessageMonitor/O2MessageMonitor.h:31,
                 from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_o2checkcode_o2/0/Utilities/O2MessageMonitor/src/O2MessageMonitor.cxx:33:
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_o2checkcode_o2/0/DataFormats/MemoryResources/include/MemoryResources/MemoryResources.h:413:32: note:   initializing argument 1 of 'o2::memory_resource::ChannelResource* o2::memory_resource::getTransportAllocator(FairMQTransportFactory*)'
 inline static ChannelResource* getTransportAllocator(FairMQTransportFactory* factory)
                                ^~~~~~~~~~~~~~~~~~~~~

mkrzewic · 2018-11-08T10:17:59Z

cannot reproduce, I suspect you're not compiling against fairmq 1.3.6 ?

sawenzel · 2018-11-08T10:20:25Z

@mkrzewic : Which version/commit of FairRoot is compatible with this? Just in order to test it locally.

mkrzewic · 2018-11-08T10:24:25Z

v1.3.6

sawenzel · 2018-11-08T10:31:07Z

@mkrzewic : I was talking about FairRoot. There doesn't seem to be a version 1.3.6 in FairRoot.

mkrzewic · 2018-11-08T10:33:32Z

ah fairroot version does not matter for this patch. However i'm using c672f280

ktf · 2018-11-08T10:33:57Z

I am fixing the SFINAE right now.

ktf · 2018-11-08T10:42:00Z

#1462 should fix the error. Once that is merged the dev-fairroot test of this one should become green.

mkrzewic · 2018-11-08T10:51:47Z

@ktf that would be great, but here i dont really see how a fix in a different, independent module would fix this?

ktf · 2018-11-08T10:54:07Z

#1462 should make my code work in both 1.2.x and 1.3.x, so if we look at o2-dev-fairroot which is basically 1.3.x, my PR + yours should be green. If that is the case we can safely move from 1.2.x to 1.3.x in alidist and forget about 1.2.x.

ktf · 2018-11-08T10:54:42Z

If you rebase and push again we should speed up the test retrial.

ktf · 2018-11-08T11:21:38Z

Ok, seems we still have a few issues with boost and ROOT...

dberzano · 2018-11-08T11:26:08Z

@ktf any idea how to solve those issues?

ktf · 2018-11-08T11:33:21Z

Nope, sorry. Maybe we need some #ifndef __cling__ in a few places?

mkrzewic · 2018-11-08T12:25:40Z

it compiles on centos7 with defaults-o2 and cmake bumped to 3.9.4 (my setup)

mkrzewic · 2018-11-08T12:26:27Z

(and of course fairmq @v1.3.6 and fairroot @c672f280)

dberzano · 2018-11-08T14:30:36Z

It does compile. Tests fail. Can you recompile (with aliBuild) after exporting ALIBUILD_O2_TESTS=1?

dberzano · 2018-11-08T23:50:32Z

We have the following error during the tests:

In file included from G__TPCSimulationDict dictionary payload:21:
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/build_O2_o2-dev-fairroot/0/Detectors/Base/include/DetectorsBase/Detector.h:30:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairMQ/dev-17/include/fairmq/FairMQChannel.h:19:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairMQ/dev-17/include/fairmq/FairMQTransportFactory.h:17:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairMQ/dev-17/include/fairmq/MemoryResources.h:21:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/boost/v1.68.0-9/include/boost/container/flat_map.hpp:29:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/boost/v1.68.0-9/include/boost/container/detail/flat_tree.hpp:29:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/boost/v1.68.0-9/include/boost/container/detail/pair.hpp:132:89: warning: invalid memory pointer passed to a callee:
static piecewise_construct_t piecewise_construct = BOOST_CONTAINER_DOC1ST(unspecified, *std_piecewise_construct_holder<>::dummy);
                                                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/boost/v1.68.0-9/include/boost/container/detail/workaround.hpp:71:46: note: expanded from macro 'BOOST_CONTAINER_DOC1ST'
#define BOOST_CONTAINER_DOC1ST(TYPE1, TYPE2) TYPE2
                                             ^~~~~

The solution to this problem does not seem so trivial, I have actually tried some #ifdef here and there as suggested but to no avail.

@mkrzewic: the code triggering ROOT's complaints is in FairMQ, and it seems to be yours. Can you have a look please?

This is essentially the only big thing that's preventing us from releasing O2 v1.0 😢

mkrzewic · 2018-11-09T00:51:38Z

looks like root cannot parse the boost headers, no idea how to solve it atm. will have a deeper look tomorrow, I'm able to reproduce this now thanks to @dberzano

sawenzel · 2018-11-09T07:16:22Z

@mkrzewic , @dberzano : I will refactor the code today to get rid of those header includes.

sawenzel · 2018-11-09T14:41:02Z

#1464 will solve the mentioned problems in detector code.

This is supposed to be a temporary measure. See AliceO2Group#1464 and AliceO2Group#1458.

This is supposed to be a temporary measure. See #1464 and #1458.

This is under protest - getDevice should NOT return a raw pointer to be adopted by a unique_ptr somewhere arbitrary. Best option is possibly to return by unique_ptr.

dberzano · 2018-11-09T15:47:19Z

With #1464 and #1465 merged, and this PR rebased (I just did it) we should finally see the o2-dev-fairroot test green, hopefully.

Note that we expect the o2 and macos tests to fail.

As soon as the test is green, we can finally merge alisw/alidist#1363 as well.

dberzano · 2018-11-09T16:39:56Z

It seems there is one last error to fix:

In file included from input_line_10:1:
/mnt/mesos/sandbox/sandbox/O2/Utilities/MCStepLogger/src/analyseSteps.C:24:82: error: no type named 'StepLookups' in namespace 'o2'
void volidTovolnameHist(std::vector<T> const &v, std::map<std::string,T> &m, o2::StepLookups *l){
                                                                             ~~~~^
/mnt/mesos/sandbox/sandbox/O2/Utilities/MCStepLogger/src/analyseSteps.C:75:6: error: no type named 'StepLookups' in namespace 'o2'
 o2::StepLookups *lookup =nullptr;
 ~~~~^
/mnt/mesos/sandbox/sandbox/O2/Utilities/MCStepLogger/src/analyseSteps.C:79:18: error: no member named 'StepInfo' in namespace 'o2'
 std::vector<o2::StepInfo> *steps = nullptr;
             ~~~~^
/mnt/mesos/sandbox/sandbox/O2/Utilities/MCStepLogger/src/analyseSteps.C:79:29: error: C++ requires a type specifier for all declarations
 std::vector<o2::StepInfo> *steps = nullptr;
                            ^
/mnt/mesos/sandbox/sandbox/O2/Utilities/MCStepLogger/src/analyseSteps.C:83:18: error: no member named 'MagCallInfo' in namespace 'o2'
 std::vector<o2::MagCallInfo> *calls = nullptr;
             ~~~~^
/mnt/mesos/sandbox/sandbox/O2/Utilities/MCStepLogger/src/analyseSteps.C:83:32: error: C++ requires a type specifier for all declarations
 std::vector<o2::MagCallInfo> *calls = nullptr;

sawenzel · 2018-11-12T09:27:43Z

I believe this new bug is not due to a problem in our code (since the same macro tested good before and is not dependent on FairMQ). Rather I suspect the problem comes from FairRoot-dev which copied the MCStepLogger from AliceO2 and which is now building the same library "MCStepLogger.so" as we do. So ROOT is probably confused. Indeed, local tests confirm this theory. Removing "libMCStepLogger.so" from FairRoot leads to a good compilation. As long as we keep the FairRoot tag, we should be good to go. I will nevertheless try to fix the problem in FairRoot dev.

dberzano · 2018-11-12T09:37:22Z

OK I came to the same conclusion and verified it as well. We indeed have libMCStepLogger.so twice, and I think the one from FairRoot should be renamed.

Let's proceed, and tag. Note, this is quite disruptive and requires bumping alidist. I'll try to do that quickly (with proper double-testing first).

sawenzel · 2018-11-12T09:41:07Z

for the record: FairRootGroup/FairRoot#846

mkrzewic mentioned this pull request Nov 7, 2018

Bump FairMQ to 1.3.6 alisw/alidist#1363

Merged

mkrzewic force-pushed the dev branch from 3282819 to 91d7793 Compare November 8, 2018 10:56

dberzano mentioned this pull request Nov 9, 2018

Hide FairMQ stuff in Detector headers #1464

Merged

dberzano pushed a commit to dberzano/AliceO2 that referenced this pull request Nov 9, 2018

Do not test macros including problematic headers

e319fcc

This is supposed to be a temporary measure. See AliceO2Group#1464 and AliceO2Group#1458.

dberzano mentioned this pull request Nov 9, 2018

Do not test macros including problematic headers #1465

Merged

dberzano pushed a commit that referenced this pull request Nov 9, 2018

Do not test macros including problematic headers

fdc8a53

This is supposed to be a temporary measure. See #1464 and #1458.

Compile against FairMQ 1.3.6

0ad3e17

mkrzewic added 2 commits November 9, 2018 16:44

Unique_ptr needs to be used here now.

a76baf1

This is under protest - getDevice should NOT return a raw pointer to be adopted by a unique_ptr somewhere arbitrary. Best option is possibly to return by unique_ptr.

clang format

24ff100

dberzano force-pushed the dev branch from 91d7793 to 24ff100 Compare November 9, 2018 15:44

dberzano merged commit 1372106 into AliceO2Group:dev Nov 12, 2018

mikesas pushed a commit to mikesas/AliceO2 that referenced this pull request Dec 13, 2022

install DataModel includes (AliceO2Group#1458)

08268de

Conversation

mkrzewic commented Nov 7, 2018

Uh oh!

sawenzel commented Nov 7, 2018

Uh oh!

sawenzel commented Nov 7, 2018

Uh oh!

dberzano commented Nov 8, 2018

Uh oh!

mkrzewic commented Nov 8, 2018

Uh oh!

sawenzel commented Nov 8, 2018

Uh oh!

mkrzewic commented Nov 8, 2018

Uh oh!

sawenzel commented Nov 8, 2018

Uh oh!

mkrzewic commented Nov 8, 2018

Uh oh!

ktf commented Nov 8, 2018

Uh oh!

ktf commented Nov 8, 2018

Uh oh!

mkrzewic commented Nov 8, 2018

Uh oh!

ktf commented Nov 8, 2018

Uh oh!

ktf commented Nov 8, 2018

Uh oh!

ktf commented Nov 8, 2018

Uh oh!

dberzano commented Nov 8, 2018

Uh oh!

ktf commented Nov 8, 2018

Uh oh!

mkrzewic commented Nov 8, 2018

Uh oh!

mkrzewic commented Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dberzano commented Nov 8, 2018

Uh oh!

dberzano commented Nov 8, 2018

Uh oh!

mkrzewic commented Nov 9, 2018

Uh oh!

sawenzel commented Nov 9, 2018

Uh oh!

sawenzel commented Nov 9, 2018

Uh oh!

dberzano commented Nov 9, 2018

Uh oh!

dberzano commented Nov 9, 2018

Uh oh!

sawenzel commented Nov 12, 2018

Uh oh!

dberzano commented Nov 12, 2018

Uh oh!

sawenzel commented Nov 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

mkrzewic commented Nov 8, 2018 •

edited

Loading