NESTCollaboration / nest

Noble Element Simulation Technique is used to simulate noble-element energy deposition microphysics.
http://nest.physics.ucdavis.edu
Other
23 stars 42 forks source link

convert all of the C-style arrays #99

Open mszydagis opened 3 years ago

mszydagis commented 3 years ago

into std::array or vectors, everywhere in the code (might be big job)

kreczko commented 3 years ago

Note: Now that you are using C++17, you might also want to look atconstexprfor all theconst` vectors/arrays (or lookup tables):

https://joelfilho.com/blog/2020/compile_time_lookup_tables_in_cpp/

mszydagis commented 3 years ago

hi @kreczko I'm confused because a while back Quentin & I already changed a ton of const to constexpr, all over the place -0- see for instance nest/include/NEST/NEST.hh (and this did not require C++17, only the old C++11 so I am extra confused!)

kreczko commented 3 years ago

constexpr in C++11 is very limited. The link above showcases the evolution through the different C++ standard.

Now you could convert things like

const std::vector<double> NESTcalc::default_NuisParam = {11., 1.1, 0.0480, -0.0533, 12.6, 0.3, 2., 0.3, 2., 0.5, 1., 1.};
const std::vector<double> NESTcalc::default_FreeParam = {1., 1., 0.1, 0.5, 0.19, 2.25}; //Fano factor of ~3 at least for ionization when using rmQuanta (look at first 2 values)

<snip>
const std::vector<double> EnergyParams = {0.23, 0.77, 2.95, -1.44};
const std::vector<double> FieldParams = {421.15, 3.27};

<snip>
//The data contained in
double modelDT[nPts][2] = {{14.6236, 24.674},
                               {24.5646, 26.4954},
                               {36.675,  29.2043},
                               {53.4802, 32.6845},
                               {74.3939, 37.9944},
                               {111.071, 45.3424},
                               {173.835, 58.2226},
                               {306.106, 74.6236},
                               {467.921, 89.2124},
                               {749.809, 96.9913},
                               {1093.38, 98.4549},
                               {1711.25, 97.3852},
                               {2615.85, 92.5493},
                               {3998.65, 85.4419},
                               {6258.24, 78.2405},
                               {9794.71, 67.7294},
                               {16453.1, 58.544},
                               {27637.8, 50.3599},
                               {38445.7, 48.5424},
                               {49828.3, 43.4695},
                               {70967.5, 36.8038},
                               {98719.8, 32.7631},
                               {127948,  29.5377},
                               {169785,  27.4412},
                               {220053,  27.9686}};
// and
double modelDL[nPts][2] = {{14.6236, 22.2977},
                               {24.5646, 22.4238},
                               {36.675,  22.933},
                               {53.4802, 23.4595},
                               {74.3939, 25.4994},
                               {111.071, 29.4632},
                               {173.835, 33.9251},
                               {306.106, 38.1019},
                               {467.921, 37.2352},
                               {749.809, 31.8425},
                               {1093.38, 25.495},
                               {1711.25, 18.3343},
                               {2615.85, 12.6461},
                               {3998.65, 8.51594},
                               {6258.24, 5.23814},
                               {9794.71, 3.01815},
                               {16453.1, 1.58294},
                               {27637.8, 0.779995},
                               {38445.7, 0.525689},
                               {49828.3, 0.364016},
                               {70967.5, 0.228694},
                               {98719.8, 0.148206},
                               {127948,  0.116139},
                               {169785,  0.0957632},
                               {220053,  0.0900259}};
mszydagis commented 3 years ago

hi @kreczko this almost worked. I got it to work on modelDL and modelDT for example, but it crapped out on EnergyParams and FieldParams (I have switched to C++17 from C++11 100% for sure here -- otherwise the others would have error, so that's not the problem). (nest-dev) [mszydagis@lux buildNEST]$ make Scanning dependencies of target NESTCore [ 5%] Building CXX object CMakeFiles/NESTCore.dir/src/NEST.cpp.o /home/mszydagis/nest/src/NEST.cpp: In member function 'virtual NEST::YieldResult NEST::NESTcalc::GetYieldERWeighted(double, double, double, const std::vector&)': /home/mszydagis/nest/src/NEST.cpp:390:35: error: the type 'const std::vector' of 'constexpr' variable 'EnergyParams' is not literal 390 | constexpr std::vector EnergyParams = {0.23, 0.77, 2.95, -1.44}; | ^~~~ In file included from /home/mszydagis/.conda/envs/nest-dev/x86_64-conda-linux-gnu/include/c++/9.4.0/vector:67, from /home/mszydagis/.conda/envs/nest-dev/x86_64-conda-linux-gnu/include/c++/9.4.0/bits/random.h:34, from /home/mszydagis/.conda/envs/nest-dev/x86_64-conda-linux-gnu/include/c++/9.4.0/random:49, from /home/mszydagis/nest/include/NEST/RandomGen.hh:12, from /home/mszydagis/nest/include/NEST/NEST.hh:65, from /home/mszydagis/nest/src/NEST.cpp:3: /home/mszydagis/.conda/envs/nest-dev/x86_64-conda-linux-gnu/include/c++/9.4.0/bits/stl_vector.h:386:11: note: 'std::vector' is not literal because: 386 | class vector : protected _Vector_base<_Tp, _Alloc> | ^~ /home/mszydagis/.conda/envs/nest-dev/x86_64-conda-linux-gnu/include/c++/9.4.0/bits/stl_vector.h:386:11: note: 'std::vector' has a non-trivial destructor /home/mszydagis/nest/src/NEST.cpp:391:35: error: the type 'const std::vector' of 'constexpr' variable 'FieldParams' is not literal 391 | constexpr std::vector FieldParams = {421.15, 3.27}; | ^~~ make[2]: [CMakeFiles/NESTCore.dir/src/NEST.cpp.o] Error 1 make[1]: [CMakeFiles/NESTCore.dir/all] Error 2 make: *** [all] Error 2

infophysics commented 2 years ago

Apparently "constexpr std::vector" is now standard in c++20, but not for earlier versions.

mszydagis commented 2 years ago

So, we are only at C++17 in NEST, and even then we only just recently upgraded from using the C++11 standard (and for good reasons: to maintain compatibility with XENON's code, especially in python, so we canNOT upgrade, at least not at this time). The code works, and runs fast, so let's just let sleeping dogs lie on this one :-) I will leave the issue open though.