open2c / cooltools

The tools for your .cool's
MIT License
140 stars 51 forks source link

Update _numutils.pyx #493

Open vishalt12345 opened 10 months ago

vishalt12345 commented 10 months ago

Changed "random" to "rand" -- The random function was not available on MSVC's "stdlib.h" for me, and after doing some more research, I realized that "rand()" instead of "random()" might be better for more compatibility across various operating systems.

sergpolly commented 10 months ago

hm , here https://stackoverflow.com/questions/76663458/do-rand-and-random-function-the-same-in-glibc they recommend using rand_s on windows ...

Should we try a system specific macro instead ? i'm not that familiar with Cython unfortunately Looks like this is the official Cython-way : https://cython.readthedocs.io/en/stable/src/userguide/external_C_code.html#including-verbatim-c-code maybe something like that ? :

cdef extern from "stdlib.h":
    """
    #if defined(_WIN32) || defined(MS_WINDOWS) || defined(_MSC_VER)
      #define _my_random()  rand_s()
    #else
      #define _my_random()  random()
    #endif
    """
    long c_libc_random "_my_random"()
    double c_libc_drandom "drand48"()
sergpolly commented 10 months ago

another suggestion/proposal - rewrite fake_cis using numba - I know it is much more than just changing import statement, but nonetheless - would you be interesting in trying this out ?

That rand function you mentioned is used only in one function inside the _numuitls.pyx module - which is fake_cis function - it doesn't seem very complicated - it received 2 2D arrays, one with data and another one with the mask (typically data would be entire hic heatmap and 2D mask would specify pixels that are cis, trans and "bad" ones to be ignored). Function simply replaces data in the cis regions with the random value from the same row - hence the name fake_cis - this is done to perform eigendecomposition on entire HiC heatmap, such that cis-data does not "interfere" with the patterns in trans data

we started discussing simplifying cython dependencies in favor of numba here https://github.com/open2c/cooltools/discussions/487