Gate C++11 about gate HOT 12 CLOSED

I want to understand the implications of point 3, without that change I am able to run Gate against c++11. So what is the point here??

from gate.

On 05/27/15 12:50, Alex Vergara Gil wrote:

I want to understand the implications of point 3, without that change
I am able to run Gate against c++11. So what is the point here??

—
Reply to this email directly or view it on GitHub
#22 (comment).

Hi Alex!

It is just a system consistency related issue. In my case the base OS
and set of basic applications is maintained centrally and distributed
across several PCs which are used for very different purposes, including
pure HEP. It just so happened, that libCLHEP was compiled without TLS,
and I am not really all that interested to know why that is so. On the
other hand, Gate uses a very particular query to estimate whether to use
TLS or not (see below). It can certainly happen that it guesses the
correct value for CLHEP_THREAD_LOCAL, and you will not see the issue I
posted. It may, however, fail. In that case, use point 3.) :-)

Quoting [source/externals/clhep/include/CLHEP/Utility/thread_local.h]

#if __cplusplus >= 201103L

#if GNUC > 4 || (GNUC == 4 && GNUC_MINOR > 7)
#define CLHEP_THREAD_LOCAL thread_local
#elif clang
#if __has_feature(cxx_thread_local)
#define CLHEP_THREAD_LOCAL thread_local
#else
#define CLHEP_THREAD_LOCAL
#endif
#else
#define CLHEP_THREAD_LOCAL
#endif

#else
#define CLHEP_THREAD_LOCAL
#endif

#endif

Cheers!

Andrej

from gate.

Ok I will use it, but since is CLHEP related I think it will be overwritten if Gate changes CLHEP version, which happens commonly.
There is also an additional issue here, in CMAKEFILES there must be a switch to enable c++11 if detected Geant4 compiled with this, is there an specific def that I must rely on?

from gate.

Maybe I can suggest that it probably will not be Geant, but ROOT that
dictates the usage of c++11, which is in use from 6.0 onwards. So I
guess checking for root-config --version might do the trick ...

Andrej

On 05/27/15 14:23, Alex Vergara Gil wrote:

Ok I will use it, but since is CLHEP related I think it will be
overwritten if Gate changes CLHEP version, which happens commonly.
There is also an additional issue here, in CMAKEFILES there must be a
switch to enable c++11 if detected Geant4 compiled with this, is there
an specific def that I must rely on?

—
Reply to this email directly or view it on GitHub
#22 (comment).

from gate.

I added an option to cmakelists to quickly and easily enable c++11 standard when compiling, it is already published on #21

from gate.

Hello both of you,

ok to make Gate compatible with c++11. To my understanding only one simple modification is required (std:: before some ifstream),. CXX flag should be left to the user yet (not c++11 by default). The CLHEP issue : we will have to update clhep embedded in Gate source. Am I right ? If so, could you please provide a pull-request with the very minimal modification to make Gate compatible to c++11 (ofstream things + cmake option from Alex).

Alex: I cannot accept your pull request #21. There are too many different changes, each of them would require careful testing. I cannot accept change (iterator, ++i) that do not solve a very explicit issue: the risk they change Gate behaviour is too high. Remember that there are a lot of applications (TEP, SPECT, RT, CT, etc) and it is really difficult to test all. To date, the tests are still manually run. There is a project of doing it automatically (see the dashboard http://my.cdash.org/index.php?project=GATE), but it is still in progress.

For accepting contributions :

1. provide pull-request with a minimal change only focus on a single issue
2. briefly describe the initial issue and the goal of the changes
3. run all benchmarks in the folder benchmarks, before and after the change, and compare results.

Sorry I don't want to dim your enthusiasm, but Gate is an "old" code used by too many people, so it is not easy to changes :)

from gate.

Dear David
if I prove all benchmarks give the same results then will you accept the changes?
There are really a hard work done over here, even with bug catching so not to bug Gate and so on.

There are other sane solution, I can provide a testing branch and you can pull it to the code, the enthusiast users can test it and if everything is fine for them then merge it to develop. Does this sound reasonable to you?

Regards

from gate.

Dear Alex,

Please note that I'm speaking as someone who is not a Gate developer
(and as someone who didn't try to understand your changes either).

Not just the code changes, but the pull request overall is very hard
to follow: "bug fixes", "merge", "yet another bugfix", "cleaning",
"yet another fix", ... This is a reasonable personal workflow, but it
would really simplify things for the developers if you could split
the changes in smaller, but consistent and self-containing pull
requests.

An example would be to create:

• a completely separate pull request with "cleaning and indentation"
  of existing code, without changing any functionality at all
• one pull request adding those "std:" prefixes (as suggested by
  Andrej) to make the code compatible with C++11
• one pull request with those three lines adding support for C++11 in
  CMake configuration files
• if bug fixes are needed in existing code, create a separate pull
  request that fixes existing bugs (ideally multiple requests for
  different bugs; using some common-sense in splitting things; there's
  no need to open 1000 pull requests to fix 1000 lines of code, but if
  you combine all bug fixes and the developers are only confident with
  fixing all but one, the won't be able to pull)
• if bug fixes were needed for your code, combine all the code changes
  and all the "fixes of previous changes" into a separate request (maybe
  together with some example demonstrating how to test the code;
  possibly an example that works after your changes and fails with the
  current implementation)
• don't include merging commits into pull requests

from gate.

Dear Mojca

As I already said, there can be a testing branch on Gate so this great pull requests can be merged without comprise Gate develop branch, users can test it (run benchmarks or personal examples) and report if the changes work or not (this case is even more important so not to work in vane).

My report is that with all those changes I can still run all benchmarks and I obtain good results from personal macros that touch different parts of the code (voxelized sources and geometries, gps sources, ion sources, "normal" geometries and so on).

Once you obtain some testing reports from enough people then you can safely merge it to develop.
Regards
Alex

from gate.

Done in #23

from gate.

As the solution for this issue is already merged I recommend to close the issue

from gate.

was not correct : change GATE_USE_STDC11 in the CMakeLists.txt
SHA: 898daf7

from gate.