Comments (19)
I also will take a look what could be alternatives in R's C API.
I don't think this is a good use of our time. See the issue I linked to (both in this thread and in the past). If the maintainers of packages like rlang
think that this is exceedingly difficult, and can't come up with an immediate replacement, then I really don't think we should take this on.
At the risk of sounding like a broken record, I'll say it one last time: I think the only reasonable way forward is to transition to using rlang
for lazy evaluation, which is an R programming task, not C. No more repetitive comments on this from me now, promise 😉
from rigraph.
https://www.stats.ox.ac.uk/pub/bdr/Intel/igraph.out
from rigraph.
from rigraph.
from rigraph.
from rigraph.
https://www.stats.ox.ac.uk/pub/bdr/Intel/igraph.out
More context in https://www.stats.ox.ac.uk/pub/bdr/Intel/igraph.log .
Haven't managed to replicate with the Intel 2023.2 compiler from https://github.com/r-hub/containers/tree/main/containers/intel .
Same problem in duckdb.
from rigraph.
Here's the warning from the Intel compiler:
In file included from vendor/cigraph/src/misc/degree_sequence.cpp:27:
In file included from /usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/algorithm:61:
In file included from /usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/stl_algo.h:61:
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/stl_tempbuf.h:263:8: warning: 'get_temporary_buffer<vd_pair>' is deprecated [-Wdeprecated-declarations]
263 | std::get_temporary_buffer<value_type>(_M_original_len));
| ^
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/stl_algo.h:4996:15: note: in instantiation of member function 'std::_Temporary_buffer<__gnu_cxx::__normal_iterator<vd_pair *, std::vector<vd_pair>>, vd_pair>::_Temporary_buffer' requested here
4996 | _TmpBuf __buf(__first, (__last - __first + 1) / 2);
| ^
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/stl_algo.h:5070:23: note: in instantiation of function template specialization 'std::__stable_sort<__gnu_cxx::__normal_iterator<vd_pair *, std::vector<vd_pair>>, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(const vd_pair &, const vd_pair &)>>' requested here
5070 | _GLIBCXX_STD_A::__stable_sort(__first, __last,
| ^
vendor/cigraph/src/misc/degree_sequence.cpp:87:18: note: in instantiation of function template specialization 'std::stable_sort<__gnu_cxx::__normal_iterator<vd_pair *, std::vector<vd_pair>>, bool (*)(const vd_pair &, const vd_pair &)>' requested here
87 | std::stable_sort(vertices.begin(), vertices.end(), degree_less<vd_pair>);
| ^
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/stl_tempbuf.h:99:5: note: 'get_temporary_buffer<vd_pair>' has been explicitly marked deprecated here
99 | _GLIBCXX17_DEPRECATED
| ^
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/x86_64-redhat-linux/bits/c++config.h:2359:34: note: expanded from macro '_GLIBCXX17_DEPRECATED'
2359 | # define _GLIBCXX17_DEPRECATED [[__deprecated__]]
| ^
As far as I can tell this is not our problem, but an issue with the combination of Intel CC and libstdc++ on that specific system. We use the std::stable_sort()
function in a standard and non-deprecated way. The internal implementation of this function uses get_temporary_buffer
, which is claimed to be deprecated, but we have no control over this. This is the internal implementation of stable_sort
.
We normally compile the C core in C++11 mode. To double-check, I compiled in C++17 mode both with GCC 13 (libstdc++) and Clang 18 (libc++), and I see no warning.
Feel free to link to this message when submitting to CRAN. I'm happy to take another look if someone can point out a specific issue with the usage of stable_sort
, but I don't believe there is any.
from rigraph.
I believe the SETLENGTH calls comes from code distributed by cpp11
, so all we need to do is to wait for that package to address the issue and recompile igraph.
from rigraph.
The PREXPR call comes code included in igraph, which seems to have been borrowed from lazyeval
. As I understand this was superseded by tidyeval
then by rlang
? Instead of including this code in igraph directly, can we use rlang
, and let them fix this on their side?
from rigraph.
@Antonov548 do you think some stuff should be fixed on igraph's side?
from rigraph.
Yes, the PREXPR issue can only be fixed on igraph's side because it's in C code that's included in igraph (basically a vendored version of https://github.com/hadley/lazyeval). However, as I said above, it may be possible to strip this out and replace it with functionality from rlang
. I don't know enough about R, and its different lazy evaluation features, to be able to tell if this is possible. But this requires R expertise, not C expertise.
from rigraph.
could you please point me to one of the uses of PREXPR?
from rigraph.
could you please point me to one of the uses of PREXPR?
Are you deeply familiar with R's C API (I'm definitely not)? If not, this is not what you should look at.
PREXPR is in src/lazyeval.c, which as I said is a vendored version of https://github.com/hadley/lazyeval
I suggest you look at what R functions R/lazyeval.R (the R counterpart of src/lazyeval.c) provides. See which of these are used and how they are used. See if these can be replaced with whatever the current rlang
provides. What I was able to figure out some weeks ago is that this definitely won't be a drop-in replacement, as the lazy evaluation strategy is said to be different. But I don't know much R and I can't handle it. Do you think you can take a look @maelle ?
In particular, see the use of lazy_dots
in make.R
and lazy_dots
/ lazy_eval
in iterators.R
. Can these be replaced with some rlang
features?
from rigraph.
PREXPR is in src/lazyeval.c, which as I said is a vendored version of https://github.com/hadley/lazyeval
I didn't see immediately your comment about vendoring of package. Good to know it's problem not only in R/igraph.
I also will take a look what could be alternatives in R's C API.
from rigraph.
I also will take a look what could be alternatives in R's C API.
I don't think this is a good use of our time. See the issue I linked to (both in this thread and in the past). If the maintainers of packages like
rlang
think that this is exceedingly difficult, and can't come up with an immediate replacement, then I really don't think we should take this on.At the risk of sounding like a broken record, I'll say it one last time: I think the only reasonable way forward is to transition to using
rlang
for lazy evaluation, which is an R programming task, not C. No more repetitive comments on this from me now, promise 😉
Make sense. I also just not sure what can be used from rlang
. Let's wait then oppnion from R experts.
from rigraph.
I am not sure when exactly but yes I know a bit of rlang, enough to look into this
from rigraph.
@maelle FYI The "non-API calls" are definitely an issue from cpp11, and must be resolved there. It's affecting a lot of packages now. PR to fix has been open for a month ,alas.
from rigraph.
thanks @mpadge!
from rigraph.
I still intend to tackle #1426... but not today.
from rigraph.
Related Issues (20)
- revdeps HOT 13
- V(g) not the same as 1:vcount HOT 5
- `cluster_label_prop()` crashes R when using fixed labels HOT 6
- Export `.from` etc. with behavior similar to `dplyr::across()` HOT 1
- use lifecyle for layout.fruchterman.reingold()
- Remove/deprecate the functions using constructor_spec()? HOT 1
- deprecate neimode argument in bfs() for good
- `feedback_arc_set()` needs tests
- Add `igraph_feedback_vertex_set()`
- A weights vector in `local_scan`
- TODOs in `rinterface_extra.c`
- TODOs in tests HOT 17
- TODO in `src/igraph_vector.hpp` HOT 2
- TODOs in `socnet.R` HOT 7
- TODO in tkplot
- ensure max.print is used consistently in all of print.R
- Update `is_separator()` docs
- can indexing be made faster HOT 1
- can we remove the "## TODO: speed this up" in make.R?
- Add `igraph_find_cycle()`
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rigraph.