ARROW-14237: [R] [CI] Disable altrep in R <= 3.5 #11404
Conversation
|
|
|
@github-actions crossbow submit test-r-versions |
|
Revision: 935a966 Submitted crossbow builds: ursacomputing/crossbow @ actions-937
|
|
Hmm, so it looks like the 3.5 is failing for partial matching issues separate from the altrep stuff. for 3.4 and below, I think I see what's happening now which is that altrep.cpp is only (fully) built if R has altrep, but then we reference a function from that in https://github.com/apache/arrow/blob/master/r/src/array_to_vector.cpp#L69, so on 3.4 altrep.hpp has nothing in it (since most/all of the file was ignored), and therefore |
|
And that partial match error might actually be related to https://bugs.r-project.org/show_bug.cgi?id=17449 |
|
Assuming that we tuned on partial matching in a recently merged PR, I guess we could turn it off for R < 3.5.0, but not sure what would make sense to do about the altrep problem. |
|
@github-actions crossbow submit test-r-versions |
|
Revision: 2f244a3 Submitted crossbow builds: ursacomputing/crossbow @ actions-938
|
|
@github-actions crossbow submit test-r-versions |
|
Revision: 1d4e7bf Submitted crossbow builds: ursacomputing/crossbow @ actions-942
|
|
@github-actions crossbow submit test-r-versions |
|
Revision: 714a35d Submitted crossbow builds: ursacomputing/crossbow @ actions-945
|
|
@github-actions crossbow submit tests-r-versions |
|
@github-actions crossbow submit test-r-versions |
|
Revision: 9517709 Submitted crossbow builds: ursacomputing/crossbow @ actions-946
|
|
@github-actions crossbow submit test-r-versions |
|
Revision: 9bc470e Submitted crossbow builds: ursacomputing/crossbow @ actions-947
|
r/src/array_to_vector.cpp
Outdated
| // try altrep first | ||
| // try altrep first | ||
| #if defined(HAS_ALTREP) | ||
| void Init_Altrep_classes(DllInfo * dll); | ||
| SEXP alt = altrep::MakeAltrepVector(chunked_array_); |
There was a problem hiding this comment.
MakeAltrepVector() is defined no matter what so that only conditional compiling would only happen in altrep.cpp and so it always returns R_NilValue when altrep is not defined, which signals that an altrep vector could not be made and so to do the normal thing
There was a problem hiding this comment.
Hmmm, these are the errors we were seeing on compilation without adding the conditional here, is there some other way to get past these without a conditional here?
array_to_vector.cpp: In member function ‘SEXPREC* arrow::r::Converter::ScheduleConvertTasks(arrow::r::RTasks&, std::shared_ptr<arrow::r::Converter>)’:
array_to_vector.cpp:69:16: error: ‘altrep’ has not been declared
SEXP alt = altrep::MakeAltrepVector(chunked_array_);
^~~~~~
array_to_vector.cpp: In member function ‘SEXPREC* arrow::r::Converter::MaybeAltrep()’:
array_to_vector.cpp:124:31: error: ‘altrep’ has not been declared
SEXP MaybeAltrep() { return altrep::MakeAltrepVector(chunked_array_); }
^~~~~~
https://github.com/ursacomputing/crossbow/runs/3885687364?check_suite_focus=true#step:7:1423
There was a problem hiding this comment.
I probably have misspend something. I'll have a look.
There was a problem hiding this comment.
I think that the bit in arrow_types.h should be:
namespace altrep {
#if defined(HAS_ALTREP)
void Init_Altrep_classes(DllInfo* dll);
#endif
SEXP MakeAltrepVector(const std::shared_ptr<ChunkedArray>& chunked_array);
} // namespace altrepi.e. the SEXP MakeAltrepVector be out of the #if defined(HAS_ALTREP)
There was a problem hiding this comment.
Line 181 in 3c12d3e
There was a problem hiding this comment.
and then there should be no need for using #if defined(HAS_ALTREP) in other places
There was a problem hiding this comment.
Thanks for these! I've managed to eliminate all (or almost all ... there was one more I want to dig in a bit more before I delete it) and it worked: https://github.com/ursacomputing/crossbow/runs/3897917054?check_suite_focus=true
Thanks again!
|
@github-actions crossbow submit test-r-versions |
|
Revision: 03506da Submitted crossbow builds: ursacomputing/crossbow @ actions-958
|
|
@github-actions crossbow submit test-r-versions |
|
@github-actions autotune |
|
Revision: 4eaa1cb Submitted crossbow builds: ursacomputing/crossbow @ actions-959
|
|
Benchmark runs are scheduled for baseline = 7eba115 and contender = cb1f15b. cb1f15b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Added a new helper function to skip tests on particular R versions