Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

convert all of the C-style arrays #99

Open
mszydagis opened this issue Apr 9, 2021 · 6 comments
Open

convert all of the C-style arrays #99

mszydagis opened this issue Apr 9, 2021 · 6 comments

Comments

@mszydagis
Copy link
Collaborator

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

@kreczko
Copy link
Contributor

kreczko commented Oct 20, 2021

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

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

@mszydagis
Copy link
Collaborator Author

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
Copy link
Contributor

kreczko commented Oct 20, 2021

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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

@mszydagis
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants