Skip to content

Conversation

@FabrizioSandri
Copy link
Owner

Description

The purpose of this pull request is to enhance the RcppDeepState harness creation procedure.
The issue I found is that the RInside library stops the test harness with the warning R is already initialized. This problem is due to the fact that during the course of an application's lifetime, no more than one RInside object can be generated. Making the RInside variable static is the solution.

@FabrizioSandri
Copy link
Owner Author

FabrizioSandri commented Jun 10, 2022

Another problem is that 'RcppDeepState_CharacterVector' uses DeepState_CStr_C and DeepState_CStrUpToLen to initialize a CharacterVector. These functions return a string. This implies that just the first character of the generated string is used in the assignments in the following code snippet.

int str_len = 20;
rand_charactervec[i] = DeepState_CStr_C(str_len, "abcdefghijklmnopqrstuvwxyz");
rand_charactervec[i] = DeepState_CStrUpToLen(str_len, "abcdefghijklmnopqrstuvwxyz");

It is preferable to construct a string of length 1 instead of generating 20 characters and taking only the first.

@FabrizioSandri
Copy link
Owner Author

The RInside part was discussed in this blog post and solved in the commit 9d7413e of the pull request #6.

@FabrizioSandri
Copy link
Owner Author

@tdhock if you agree with me I merge these changes into master. This pull request is meant to solve some minor bugs inside the RcppDeepState.h header file and to fix the R is already initialized error. The latter has been improved in the commit 9d7413e of the pull request #6 in order to work with seeds.

@tdhock
Copy link
Collaborator

tdhock commented Jun 30, 2022

I'm not sure I understand the string issue, can you please clarify?

@FabrizioSandri
Copy link
Owner Author

I got confused by the C/C++ notation where a character vector is a vector composed of single characters. However I can see that in R a character vector is composed by strings. I revert that change.

@FabrizioSandri
Copy link
Owner Author

In the last commit I removed the rand_size variable inside RcppDeepState_IntegerVector(int size,int low,int high) because the size is already passed to the function, so there's no need to use a random size.

@FabrizioSandri
Copy link
Owner Author

I also removed the rand_size variable in the RcppDeepState_NumericVector(int size,int low,int high) function.

@tdhock
Copy link
Collaborator

tdhock commented Jun 30, 2022

in C++ Rcpp::CharacterVector ch_vec(2); has elements ch_vec[0], ch_vec[1] both of which are strings.
Each string is a null-terminated sequnce of characters.

@FabrizioSandri
Copy link
Owner Author

Thank you @tdhock for the clarification.
I mistakenly believed that the syntax Rcpp::CharacterVector ch_vec(2); was equivalent to vector<char> ch_vec(2);.
The latter creates a vector of two characters, whereas the first creates two instances of Rcpp::CharacterVector, resulting in two strings.

I don't understand this naming choice of Rcpp, in fact when I write Rcpp::NumericVector v(3); I am declaring a vector of three integers.

Is it possible that the lack of a distinction between a character and a string in R is the cause of this?

@tdhock
Copy link
Collaborator

tdhock commented Jul 6, 2022

clarifications:

  • Rcpp::CharacterVector ch_vec(2); is analogous to std::vector<std::string> vec_of_strings; but R character vectors have a fixed size (here 2) whereas STL vectors can change in size.
  • "the first creates two instances of Rcpp::CharacterVector" no, not two instances. in C++ we actually say this is a single instance of the Rcpp::CharacterVector class, which represents a vector of several strings (here 2).
  • "when I write Rcpp::NumericVector v(3); I am declaring a vector of three integers." actually double precision numbers not integers.
  • In R the analog for the char C type is a CharacterVector with one element. Analog for std::string C++ type is also a character vector with one element.

@FabrizioSandri
Copy link
Owner Author

Thank you for the clarification, @tdhock. Now I can see the distinction. I close this pull request.

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

Successfully merging this pull request may close these issues.

3 participants