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

Try&Catch construct for mrcpp::get_func #87

Open
ilfreddy opened this issue Feb 16, 2019 · 11 comments
Open

Try&Catch construct for mrcpp::get_func #87

ilfreddy opened this issue Feb 16, 2019 · 11 comments

Comments

@ilfreddy
Copy link
Contributor

ilfreddy commented Feb 16, 2019

I would like to implement this feature: [www.cplusplus.com/reference/stdexcept/out_of_range/] for the mrcpp::get_func of FunctionTreeVector.
It would be nice to have such a feature for the XCFunctional stuff, now that I am heavily making use of FunctionTreeVector-s for densities and their gradients.
Would that be OK?

@robertodr
Copy link
Contributor

This should be already available if you access an element of the vector with at() rather than []. It incurs a performance penalty, as it needs to check bounds at every access.

@ilfreddy
Copy link
Contributor Author

That was also my point. How much should we be concerned with performance at this level? We are talking about a vector of tree structures containing a handful of elements (essentially FunctionTree pointers).

@ilfreddy
Copy link
Contributor Author

Regarding the example in the link, would it be working if I remove the assignment = 100 in the try statement? In other words, I just want to see if the element is correctly defined, not to assign a value.
@robertodr what was the link of the sandbox you showed us?

@robertodr
Copy link
Contributor

The website is coliru. You need to do something with the result of at() either assign it to a dummy variable or assign a new value to it. How many of this checked accesses you'd need? Why not if (idx > vec.size()) abort()? This should have lower overhead and avoid littering the code with try-catch.

@ilfreddy
Copy link
Contributor Author

if I place the at() in mrcpp::get_func(), it is only going to be there. But fair enough it is going to be used EVERY TIME. I think I can supersede, also because I do not necessarily need to check for bounds only. Sometimes I need to check that the vector contains the correct number of elements before appending a new one. I guess I need to consult @stigrj to find a good strategy.

@robertodr
Copy link
Contributor

Is this specific to XCFunctional? Can you calculate all these bounds at construction? You can add checks on preconditions and postconditions when entering and before return-ing in the methods of the class.

@robertodr
Copy link
Contributor

Even better, if you can templatize over these conditions you'd like to check. Then it's the compiler checking everything makes sense.

@stigrj
Copy link
Contributor

stigrj commented Feb 16, 2019

I think you can safely ignore performance at this level

@ilfreddy
Copy link
Contributor Author

Yes. It's pretty XCFunctional specific. The class needs to know the densities (unperturbed and perturbed to the nth order for nth-order response calculations). They are built in different parts of the code and then passed to XCFunctional which is then feeding them to XCFun to compute the potentials. So I need to know that when I append a given density to the vector, the vector itself is "sane" (e.g. it contains all lower-order densities).

Concerning templetizing... thanks but my C++ level is well below what you would like me to do here, I'm afraid... :-(

@robertodr
Copy link
Contributor

What I was also trying to say is that in a month or two, you'll have forgotten why you're checking that vector has length 15.

@ilfreddy
Copy link
Contributor Author

Agree. The current "hack" (awaiting for Stig's counseling) is to pass the index where I want the fcn to be placed. The vector size has to be exactly that index, otherwise I abort. Not pretty, but safe :-)

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