Comments (12)
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 :
- provide pull-request with a minimal change only focus on a single issue
- briefly describe the initial issue and the goal of the changes
- 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.
Related Issues (20)
- CT imaging on a mesh model HOT 1
- SPECT fanbeam collimator HOT 2
- Export of CT-data
- Infinite loop/stuck particle in voxelized phantoms with emstandard_opt4 and GATE v9.3 HOT 3
- gjs executable crash - Gate9 HOT 4
- Problems after vGate9.3 installation HOT 2
- Issue with GateVImageVolume.cc (l.734)
- Ubuntu 20.04 Snap Version of Gate Permission Issues HOT 1
- Issue with GateSourcePhaseSpace.cc if the source is in ROOT format after installing V9.3
- Image layers on docker image for Gate9.3 doesn't match with script versions
- Attach actor to "world" volume causing summary to malfuntion. HOT 13
- Whether Gate can output the energy information of secondary particles?
- segmentation fault when using SinglesDigitizer and ROOT output HOT 1
- separating slash in branch name hinders easy plotting HOT 2
- for pPs + prompt gamma simulation with Gate
- material issue!
- setSkipEqualMaterials of ImageRegularParametrisedVolume setting to zero causing system to complaining geometry overlapping
- /gate/source/F-18/imageReader/linearTranslator/setScale 1 MBq
- Directional Keys Not Working in Gate Software
- Creating a circular source in Gate
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gate.