special handling of linear binning

Hi,
As discussed with Lehman for DESI applications, this is a first attempt to implement a special handling of linear binning, which helps corrfunc run faster with a large number of bins.
There is no speed gain at low number of bins ~ 10, but a speed gain > 3x for bins ~ 200.
I added a flag bin_type to the Python wrapper, to choose between: 'auto', 'lin', 'custom'; with 'lin' for linear binning, 'auto' (default) to automatically detect linear binning
(which happens here: https://github.com/adematti/Corrfunc/blob/c500d2c9137ff7f66d34eb4189bc19f9325d84ff/utils/utils.c#L85).
The following pair counters have been updated:
mocks: DDrppi_mocks, DDsmu_mocks
theory: DD, DDrppi, DDsmu, xi, wp
I did not update DDtheta_mocks (yet), as there are implementation choices to be made:
a) should I consider the binning to be linear in theta or acos? (I guess the former, as the pair counts for the latter can be obained from DD if maximum separation < \pi)
b) should I use fast acos to compute the bin for a given pair? in this case, a pair can fall in the wrong bin due to numerical approximation
To be discussed, also:

should I pass rstep (bin width), rmin to the kernels directly (currently taking the square root of sqr_rmin, sqr_rmax, which is slightly suboptimal)
should I print the chosen binning type; in this case, where? in each countpairs_DOUBLE function?
should I leave option 'auto'? it checks for linear binning, with absolute and relative tolerance of 1e-12 for both double and floats.
Not sure how it behaves in practice for the latter case (floats), see remark ii) below.
I also have a couple of remarks:
i) this line https://github.com/manodeep/Corrfunc/blob/596fe77078d59b296b34608927e301c427331919/Corrfunc/utils.py#L461 seems unnecessary?
(as array updates are done in place and calls to this functions do not retrieve the returned arrays)
ii) I don't think setup_bins_float is used anywhere in the code, and setup_bins_double is only used here: https://github.com/manodeep/Corrfunc/blob/74c6fc29f9a0236eaebbc1830a1f59fd9a53bfc8/mocks/DDtheta_mocks/countpairs_theta_mocks_impl.c.src#L534
Is there any reason not to use a single version of this function, setup_bins?
iii) Some time ago I had binning errors with Corrfunc after calling matplotlib. I figured out this was due to how the string separation for floats was handled (matplotlib changed the environment variables specifying this convention;
export LC_NUMERIC=C solved this.). For this issue not to happen to other people, I guess the simplest solution would be to pass bins as an array rather than a file to be read from disk. Is there any reason not to do so?

Thanks!
Best,
Arnaud

Hello @adematti! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file Corrfunc/mocks/DDrppi_mocks.py:

Line 26:80: E501 line too long (88 > 79 characters)
Line 223:80: E501 line too long (80 > 79 characters)
Line 334:80: E501 line too long (96 > 79 characters)

In the file Corrfunc/mocks/DDsmu_mocks.py:

Line 25:80: E501 line too long (86 > 79 characters)
Line 225:80: E501 line too long (80 > 79 characters)
Line 255:80: E501 line too long (96 > 79 characters)

In the file Corrfunc/tests.py:

Line 35:80: E501 line too long (96 > 79 characters)
Line 48:19: E231 missing whitespace after ','
Line 61:28: E231 missing whitespace after ','
Line 61:33: E231 missing whitespace after ','
Line 62:22: E231 missing whitespace after ','
Line 63:16: E231 missing whitespace after ','
Line 63:37: E231 missing whitespace after ','
Line 74:42: E127 continuation line over-indented for visual indent
Line 75:42: E127 continuation line over-indented for visual indent
Line 76:42: E127 continuation line over-indented for visual indent
Line 76:80: E501 line too long (80 > 79 characters)
Line 77:42: E127 continuation line over-indented for visual indent
Line 77:80: E501 line too long (101 > 79 characters)
Line 79:42: E127 continuation line over-indented for visual indent
Line 80:42: E127 continuation line over-indented for visual indent
Line 81:42: E127 continuation line over-indented for visual indent
Line 81:80: E501 line too long (80 > 79 characters)
Line 82:42: E127 continuation line over-indented for visual indent
Line 82:80: E501 line too long (99 > 79 characters)
Line 83:35: E231 missing whitespace after ','
Line 90:40: E127 continuation line over-indented for visual indent
Line 91:40: E127 continuation line over-indented for visual indent
Line 92:40: E127 continuation line over-indented for visual indent
Line 93:40: E127 continuation line over-indented for visual indent
Line 93:80: E501 line too long (94 > 79 characters)
Line 95:40: E127 continuation line over-indented for visual indent
Line 96:40: E127 continuation line over-indented for visual indent
Line 97:40: E127 continuation line over-indented for visual indent
Line 98:40: E127 continuation line over-indented for visual indent
Line 98:80: E501 line too long (91 > 79 characters)
Line 99:34: E231 missing whitespace after ','
Line 103:80: E501 line too long (96 > 79 characters)
Line 114:19: E231 missing whitespace after ','
Line 127:28: E231 missing whitespace after ','
Line 127:33: E231 missing whitespace after ','
Line 128:22: E231 missing whitespace after ','
Line 129:16: E231 missing whitespace after ','
Line 129:37: E231 missing whitespace after ','
Line 139:29: E127 continuation line over-indented for visual indent
Line 140:29: E127 continuation line over-indented for visual indent
Line 141:29: E127 continuation line over-indented for visual indent
Line 143:29: E127 continuation line over-indented for visual indent
Line 144:29: E127 continuation line over-indented for visual indent
Line 145:29: E127 continuation line over-indented for visual indent
Line 146:24: E231 missing whitespace after ','
Line 150:29: E128 continuation line under-indented for visual indent
Line 151:29: E128 continuation line under-indented for visual indent
Line 152:29: E128 continuation line under-indented for visual indent
Line 153:29: E128 continuation line under-indented for visual indent
Line 159:28: E231 missing whitespace after ','
Line 166:37: E127 continuation line over-indented for visual indent
Line 167:37: E127 continuation line over-indented for visual indent
Line 168:37: E127 continuation line over-indented for visual indent
Line 169:37: E127 continuation line over-indented for visual indent
Line 170:37: E127 continuation line over-indented for visual indent
Line 171:37: E127 continuation line over-indented for visual indent
Line 173:37: E127 continuation line over-indented for visual indent
Line 174:37: E127 continuation line over-indented for visual indent
Line 175:37: E127 continuation line over-indented for visual indent
Line 176:37: E127 continuation line over-indented for visual indent
Line 177:37: E127 continuation line over-indented for visual indent
Line 178:37: E127 continuation line over-indented for visual indent
Line 179:27: E231 missing whitespace after ','
Line 183:29: E127 continuation line over-indented for visual indent
Line 184:29: E127 continuation line over-indented for visual indent
Line 185:29: E127 continuation line over-indented for visual indent
Line 187:29: E127 continuation line over-indented for visual indent
Line 188:29: E127 continuation line over-indented for visual indent
Line 189:29: E127 continuation line over-indented for visual indent
Line 190:24: E231 missing whitespace after ','
Line 194:29: E127 continuation line over-indented for visual indent
Line 195:29: E127 continuation line over-indented for visual indent
Line 196:29: E127 continuation line over-indented for visual indent
Line 198:29: E127 continuation line over-indented for visual indent
Line 199:29: E127 continuation line over-indented for visual indent
Line 200:29: E127 continuation line over-indented for visual indent
Line 201:24: E231 missing whitespace after ','
Line 207:27: E231 missing whitespace after ','
Line 207:35: E231 missing whitespace after ','
Line 207:41: E231 missing whitespace after ','

In the file Corrfunc/theory/DD.py:

Line 144:80: E501 line too long (80 > 79 characters)
Line 213:80: E501 line too long (96 > 79 characters)

In the file Corrfunc/theory/DDrppi.py:

Line 23:80: E501 line too long (82 > 79 characters)
Line 161:80: E501 line too long (80 > 79 characters)
Line 259:80: E501 line too long (96 > 79 characters)

In the file Corrfunc/theory/DDsmu.py:

Line 24:80: E501 line too long (81 > 79 characters)
Line 169:80: E501 line too long (80 > 79 characters)
Line 265:80: E501 line too long (96 > 79 characters)

In the file Corrfunc/theory/wp.py:

Line 411:80: E501 line too long (80 > 79 characters)
Line 489:80: E501 line too long (96 > 79 characters)

In the file Corrfunc/theory/xi.py:

Line 130:80: E501 line too long (80 > 79 characters)
Line 199:80: E501 line too long (96 > 79 characters)

In the file Corrfunc/utils.py:

Line 18:13: E127 continuation line over-indented for visual indent
Line 18:80: E501 line too long (99 > 79 characters)
Line 551:29: E231 missing whitespace after ','
Line 551:38: E231 missing whitespace after ','

@adematti Great - thanks for the PR. I took a quick look at some C files and can see a couple of improvements. Quickly leaving comments for now - will add a full code review later.

It might be better to rename the new function setup_bin_type to detect_bin_type
What happens when there is exactly 1 bin - the current setup will end up with a divide by zero
In the kernels, you are likely to get better performance by computing the reciprocal of rpstep and then multiplying with that reciprocal (in both the SIMD and the regular trailing loop sections)
Will you please check if it is possible to cast the linear histogram bin calculation [truncate((r - rmin)*inv_rstep)] as a fused multiply-add (FMA)? If so, that might save a bit more cpu cycles

Hi,
As discussed with Lehman for DESI applications, this is a first attempt to implement a special handling of linear binning, which helps corrfunc run faster with a large number of bins.
There is no speed gain at low number of bins ~ 10, but a speed gain > 3x for bins ~ 200.
I added a flag bin_type to the Python wrapper, to choose between: 'auto', 'lin', 'custom'; with 'lin' for linear binning, 'auto' (default) to automatically detect linear binning
(which happens here: https://github.com/adematti/Corrfunc/blob/c500d2c9137ff7f66d34eb4189bc19f9325d84ff/utils/utils.c#L85).
The following pair counters have been updated:
mocks: DDrppi_mocks, DDsmu_mocks
theory: DD, DDrppi, DDsmu, xi, wp
I did not update DDtheta_mocks (yet), as there are implementation choices to be made:
a) should I consider the binning to be linear in theta or acos? (I guess the former, as the pair counts for the latter can be obained from DD if maximum separation < \pi)

I suspect linear in theta is the more typical choice. Is it possible to support both through some user-option?

b) should I use fast acos to compute the bin for a given pair? in this case, a pair can fall in the wrong bin due to numerical approximation
To be discussed, also:

Hmmm - fast-acos option should be independent of the bin-type choice. If the user has specified fast_acos, then we should the approximations, otherwise, we should use the full-accuracy trig function

1. should I pass rstep (bin width), rmin to the kernels directly (currently taking the square root of sqr_rmin, sqr_rmax, which is slightly suboptimal)

Yeah - I saw that one. May be change the behaviour to pass rmin and rmax to the kernels, and then square those to get the sqr_rmin and sqr_rmax`?

2. should I print the chosen binning type; in this case, where? in each countpairs_DOUBLE function?

Yes, but only if options->verbose is set.

3. should I leave option 'auto'? it checks for linear binning, with absolute and relative tolerance of 1e-12 for both double and floats.
   Not sure how it behaves in practice for the latter case (floats), see remark ii) below.

I would change the tolerance to be appropriate for the precision of the numeric type. 1e-12 seems reasonable for double but too small for float (probably more like 1e-8). May be check what the default options for np.allclose are?

   I also have a couple of remarks:
   i) this line https://github.com/manodeep/Corrfunc/blob/596fe77078d59b296b34608927e301c427331919/Corrfunc/utils.py#L461
    seems unnecessary?
   (as array updates are done in place and calls to this functions do not retrieve the returned arrays)

This is for @lgarrison

   ii) I don't think setup_bins_float is used anywhere in the code, and setup_bins_double is only used here: https://github.com/manodeep/Corrfunc/blob/74c6fc29f9a0236eaebbc1830a1f59fd9a53bfc8/mocks/DDtheta_mocks/countpairs_theta_mocks_impl.c.src#L534

The source (.c.src files) are pre-processed by sed twice and DOUBLE becomes double and float -- i.e., setup_bins_DOUBLE translates to both setup_bins_float and setup_bins_double

   Is there any reason not to use a single version of this function, setup_bins?
   iii) Some time ago I had binning errors with Corrfunc after calling matplotlib. I figured out this was due to how the string separation for floats was handled (matplotlib changed the environment variables specifying this convention;
   export LC_NUMERIC=C solved this.). For this issue not to happen to other people, I guess the simplest solution would be to pass bins as an array rather than a file to be read from disk. Is there any reason not to do so?

Huh - okay! I can see that happening but that's poor behaviour on maplotlib's part.
The reason for specifying the bins from a file is historical - reading from a file lets us keep the exact same bins as the output from a previous Corrfunc run.
Many thanks for using Corrfunc and adding such a feature! Hopefully, I have answered all/most of the queries.
Cheers,
Manodeep

Thanks!
Best,
Arnaud

* Will you please check if it is possible to cast the linear histogram bin calculation `[truncate((r - rmin)*inv_rstep)]` as a fused multiply-add (FMA)? If so, that might save a bit more cpu cycles

https://software.intel.com/sites/landingpage/IntrinsicsGuide/#avx512techs=AVX512F&expand=3441,3186&text=mm512_mask3_fmadd_round_p
If you rewrite the bin calculation from  rpbin := (r - rmin)*inv_rstep to rpbin := r*inv_rstep - rmin*inv_rstep, then the calculation takes the required FMA form. The rounding parameter in the intrinsic is likely the _MM_FROUND_TO_ZERO |_MM_FROUND_NO_EXC but might require a separate truncate to integer.

Thanks for your answer!
Having one bin (nbin = 2) is ok; zero bin (nbin < 2) fails after checks like https://github.com/manodeep/Corrfunc/blob/74c6fc29f9a0236eaebbc1830a1f59fd9a53bfc8/mocks/DDsmu_mocks/countpairs_s_mu_mocks_impl.c.src#L302
I can have detect_bin_type return an EXIT_FAILURE in case nbin <= 1 (though that will have no impact on the code).
Concerning kernels and FMA, I will check that asap.

I suspect linear in theta is the more typical choice. Is it possible to support both through some user-option?

hum, like "cos_lin"? but that will only make sense for DDtheta...

Hmmm - fast-acos option should be independent of the bin-type choice. If the user has specified fast_acos, then we should the approximations, otherwise, we should use the full-accuracy trig function

My point is that using fast-acos to perform linear binning may yield different npairs than with the "custom" binning (fast-acos being an approximation). I don't think we want that. So linear binning should rely on true-acos, whatever is required for thetaavg. I guess that will degrade efficiency by a lot - though not tested yet.
By the way, "auto" is used by default, meaning linear binning whenever input bins are linear. My tests showed it did not slow down pair counts even with 10 bins, but these tests are not exhaustive...

May be check what the default options for np.allclose are?

This is rtol=1e-05, atol=1e-08. It may yield incorrect pair counts. Then I guess we should have "custom" as a default instead of "auto", as pair counts should be guaranteed correct with the default options.

The source (.c.src files) are pre-processed by sed twice and DOUBLE becomes double and float -- i.e., setup_bins_DOUBLE translates to both setup_bins_float and setup_bins_double

hm... setup_bins_float and setup_bins_double are defined in utils.c. There is no utils.c.src, hence no setup_bins_DOUBLE definition. All the calls are to setup_bins (except for DDtheta, where setup_bins_double in called). Maybe I missed something?

Hey @adematti, thank you so much for this contribution! This is not a small effort, and it looks great so far. And thanks @manodeep for reviewing this while I was away.  I will try to catch up on the full set of comments above and do my own review, but probably not until tomorrow or Monday at the earliest.

Thanks for your answer!
Having one bin (nbin = 2) is ok; zero bin (nbin < 2) fails after checks like

Great! I always forget that the way the code is setup nbins in the code is not the usual nbins  from the user-perspective

https://github.com/manodeep/Corrfunc/blob/74c6fc29f9a0236eaebbc1830a1f59fd9a53bfc8/mocks/DDsmu_mocks/countpairs_s_mu_mocks_impl.c.src#L302
I can have detect_bin_type return an EXIT_FAILURE in case nbin <= 1 (though that will have no impact on the code).
Concerning kernels and FMA, I will check that asap.

Awesome - yup!

I suspect linear in theta is the more typical choice. Is it possible to support both through some user-option?

hum, like "cos_lin"? but that will only make sense for DDtheta...

Or better yet, we can design for the ability to allow cos_lin in the future but only implement theta_lin for now.

Hmmm - fast-acos option should be independent of the bin-type choice. If the user has specified fast_acos, then we should the approximations, otherwise, we should use the full-accuracy trig function

My point is that using fast-acos to perform linear binning may yield different npairs than with the "custom" binning (fast-acos being an approximation). I don't think we want that. So linear binning should rely on true-acos, whatever is required for thetaavg. I guess that will degrade efficiency by a lot - though not tested yet.

If I am understanding correctly, then you are referring to the impact of fast_acos during the detection of the bin type. If so, yes, I agree - the bin detection should run with the full precision acos. The impact of fast_acos is only in the kernels where the actual calculations are done.

The source (.c.src files) are pre-processed by sed twice and DOUBLE becomes double and float -- i.e., setup_bins_DOUBLE translates to both setup_bins_float and setup_bins_double

hm... setup_bins_float and setup_bins_double are defined in utils.c. There is no utils.c.src, hence no setup_bins_DOUBLE definition. All the calls are to setup_bins (except for DDtheta, where setup_bins_double in called). Maybe I missed something?

Most of the functions in utils.c.src are not precision dependent - hence we did not have the two versions. However, that can certainly change if the need arises.

clang tests fail because the -mfma flag is required.
gcc tests fail because of this line (which I left untouched):
https://github.com/manodeep/Corrfunc/blob/596fe77078d59b296b34608927e301c427331919/theory/DD/countpairs_kernels.c.src#L35
I guess one could remove it?
I very naively tried compiling on Cori KNL, after loading craype-mic-knl, but install did not pick the AVX512 flag,
I guess there are other things to specify?

clang tests fail because the -mfma flag is required.

Hmm, would have hoped -march=native would have set that if the CPU supports it.  The CI CPUs probably support it, as its quite old.  But if it's a weird virtual environment they might not, in which case the correct behavior would be to protect the SSE kernels with an FMA check.
We should probably add a lscpu command to the GitHub CI so we can triage cases like this.  Will PR this...

gcc tests fail because of this line (which I left untouched):
https://github.com/manodeep/Corrfunc/blob/596fe77078d59b296b34608927e301c427331919/theory/DD/countpairs_kernels.c.src#L35
I guess one could remove it?

Yeah, should remove that line now that those variables are not part of the function signature.  And actually, sqr_rpmin below that can be removed completely, as its unused.

I very naively tried compiling on Cori KNL, after loading craype-mic-knl, but install did not pick the AVX512 flag,
I guess there are other things to specify?

I guess so. I'll try it myself once I fix a quota issue at NERSC!

Hi @adematti, can you merge the latest master into your feature branch and push so the tests are rerun with the new CI config? That will also dump the CPU flags so we can debug the OSX failure, although my preliminary look at #259 suggests that the OSX CI CPUs indeed have SSE but no FMA, and since this branch introduces an FMA requirement, it breaks.  So, we could require that both FMA and SSE are present; that shouldn't be too burdensome, as FMA is quite old (2011/2012).

Here's a better solution, I think: in sse_calls.h, can you just alias the call to a non-FMA version if FMA isn't available? i.e.:
#ifdef __FMA__
#define SSE_FMA_ADD_FLOATS(X,Y,Z)          _mm_fmadd_ps(X,Y,Z)
#else
#define SSE_FMA_ADD_FLOATS(X,Y,Z)          _mm_add_ps(_mm_mul_ps(X,Y),Z)
#endif

I see, thanks! I read the FMA was introduced at the same time as AVX2, hence wrongly thought FMA flag was defined for AVX as well. I will remove changes to avx_calls.h.

Removing the FMA request for AVX raises again errors like:
'countpairs_avx_intrinsics_double' that is compiled without support for 'fma'
                m_rpbin = AVX_FMA_ADD_FLOATS(union_mDperp.m_Dperp,m_inv_rpstep,m_rpmin_invstep); // r2, then union_mDperp.m_Dperp aready clipped

(there are also some more bugs with AVX512, not unexpected as I could not try compiling on my computer)

Okay! You were right the first time, then; feel free to add the check back to AVX too.
For what it's worth, if you're trying to check if AVX-512 is compiling properly, you can probably hack it to compile on your computer by replacing -march=native with -march=skylake-avx512 or similar. You won't be able to run it, but it ought to compile.

For what it's worth, if you're trying to check if AVX-512 is compiling properly, you can probably hack it to compile on your computer by replacing -march=native with -march=skylake-avx512 or similar. You won't be able to run it, but it ought to compile.

Yup - let's make sure about the code compiling. The GH actions cpus have AVX512F - so pushing should automatically test those new kernels.
I do have access to skylake cpus - so will setup a comprehensive testing when I get a chance.

Doesn't appear so; @adematti, can you merge master into linearbinning?

@adematti Here is my attempt at a workaround: https://github.com/manodeep/Corrfunc/tree/linearbinning-rounding
Can you merge that into your branch?
It's possible switching the whole bin index computation to ints would be more efficient, but the int AVX512 macros are not set up in the way we would want them to be regarding type widths, I think. So this is the easiest solution for now.
We ought to assess the performance impact of the extra rounding, since it's kind of expensive (8 cycles).  Hopefully the linear bin computation will more than make up for that.

done! sorry for the delay

Thanks for checking this. Did you run pip install -e ./ before running python -m Corrfunc.tests? Otherwise, it may be picking up an old version of the Python extension.

Yeah, definitely more than a few pairs.  I haven't been able to reproduce... can you share your compiler version?

Great, thanks. Unfortunately, I will probably not have time to investigate this until next week. But if you or @adematti feel inspired to do some debugging, please go ahead!

I tried to reproduce reproduce @manodeep's issue using gcc/9.3.0 (I don't have easy access to 9.2.0), but wasn't able to.  I ran the linear binning under the clang address/undefined/memory sanitizers, but I didn't find any issues.  However, the executable I tested was theory/tests/test_periodic (modified with options.bin_type=BIN_LIN;), and I'm not sure if that executable exhibits @manodeep's issue. Sanitizing the Python extensions is hard because of all the false positives from Python!
I did find a few warnings from the thread sanitizer, but none of them seem related to this issue. I couldn't make sense of all of the messages, but at least one of them did seem to be real, albeit probably not in a way that affected anything. I've pushed a fix for that, just in case.
So... any ideas on the next steps? @manodeep, just to confirm, the issue is deterministic and happens for any number of threads?

Yup the failure is deterministic - fails the first time every time. I just tried with nthreads=1, 2, 3 and the tests failed in all cases

@manodeep to aid the debugging, I've created a stripped-down test in theory/tests/test_periodic_lin.c.  It runs linear binning and compares against the custom-binning answer. It passes for me, but my hope is that it will fail for you and that you might be able to try simple variations of it to narrow down the cause. It's also simple enough that the sanitizers may yield useful information.  Would you mind trying it out?  You can run it with make test_periodic_lin.

That test passes! Uses the AVX512 kernel too (at least according to output)
-------COMPILE SETTINGS------------
         MAKE            = ["make"]
         CC              = ["gcc"]
         OPT             = ["-DPERIODIC -DENABLE_MIN_SEP_OPT  -DCOPY_PARTICLES  -DUSE_OMP"]
         CFLAGS          = [" -DVERSION=\"2.4.0\" -DUSE_UNICODE -std=c99 -m64 -g -Wsign-compare -Wall -Wextra -Wshadow -Wunused -fPIC -D_POSIX_SOURCE=200809L -D_GNU_SOURCE -D_DARWIN_C_SOURCE -O3  -ftree-vectorize -funroll-loops -fprefetch-loop-arrays --param simultaneous-prefetches=4  -fopenmp -funroll-loops -march=native -fno-strict-aliasing -Wformat=2  -Wpacked  -Wnested-externs -Wpointer-arith  -Wredundant-decls  -Wfloat-equal -Wcast-qual -Wcast-align -Wmissing-declarations -Wmissing-prototypes  -Wnested-externs -Wstrict-prototypes   -Wno-unused-local-typedefs "]
         CLINK           = [" -lrt  -fopenmp -lm"]
         PYTHON          = ["python"]
         GSL_CFLAGS      = ["-I/apps/skylake/software/GSL/2.5-GCC-9.2.0/include"]
         GSL_LINK        = ["-L/apps/skylake/software/GSL/2.5-GCC-9.2.0/lib -lgsl -lgslcblas -lm -Xlinker -rpath -Xlinker /apps/skylake/software/GSL/2.5-GCC-9.2.0/lib"]
         PYTHON_CFLAGS   = ["-isystem/apps/skylake/software/Python/3.8.5-gni-2020.0/include/python3.8 -isystem /apps/skylake/software/numpy/1.19.2-gni-2020.0-Python-3.8.5/lib/python3.8/site-packages/numpy-1.19.2-py3.8-linux-x86_64.egg/numpy/core/include/numpy/"]
-------END OF COMPILE SETTINGS------------

./test_periodic_lin
WARNING: File was written in a different precision than requested (file precision = 4 requested precision = 8)
Linear binning
Running with points in [xmin,xmax] = 0.000400,419.999817 with periodic wrapping = 420.000000
Running with points in [ymin,ymax] = 0.000100,419.999512 with periodic wrapping = 420.000000
Running with points in [zmin,zmax] = 0.000300,419.999207 with periodic wrapping = 420.000000
In gridlink_double> Running with [nmesh_x, nmesh_y, nmesh_z]  = 52,52,52.  Time taken =   0.107 sec
Using AVX512 kernel
0%.........10%.........20%.........30%.........40%.........50%.........60%.........70%.........80%.........90%.........100% done. Time taken =  0.620 secs
PASSED: Mr19 DD (periodic). Time taken =     0.87 seconds
PASSED: ALL 1 tests. Total time =     0.93 seconds

Darn... do you think you can try some variations to get it to fail? E.g. different kernels, number of bins, number of data points, auto vs cross-corr?  Maybe the fastest way to get it to fail would be to write the exact particles used by the Python tests.py script to disk, and have this test_periodic_lin test load them instead of the Mr19 particles.

Yesterday, I tested with -DINTEGRATION_TESTS and I got a illegal signal (SIGILL) for the SSE kernel(fallback kernel works fine) - that should not have happened. I can't reproduce that SIGILL when I add -DINTEGRATION_TESTS to the top of the source test_periodic_lin.c file, rather than to common.mk. From what I can tell, that's happening because of the sanitize options that get added into the compile line.
To check my sanity and any potential for user-error: make tests_bin passes but running python -m Corrfunc.tests immediately after,  fails.

@lgarrison Did you want me to merge in my updates from this PR into the linearbinning-rounding branch and then we can work on that branch? Otherwise, I can create a new branch and push to that

Actually I think we can just work in this branch (as long as @adematti doesn't mind!). I think the "Allow edits by maintainers" option is on; I've been able to push to this branch with no problems.
But if you have updates that logically should go in another branch then that's totally fine too.

Alright changes pushed. The new tests fail on the supercomputer - I am curious to see what happens on the CI

The new tests do fail on the CI, but only by 4 pairs. I think this is an "ordinary" failure due to floating-point precision, and not the gcc/9.2 failure, which was by thousands of pairs. When you run these tests locally, how many pairs do they fail by?

Thanks to the excellent local scientific computing staff, I was able to get a gcc/9.2 binutils/2.33.1 toolchain to test with. Unfortunately, it did not exhibit the bug.
One suggestion that came up in conversation with them: gcc might be emitting some SIMD instructions even without -march=native. Can you try -march=x86-64 or something equally innocuous?

Okay, I've made some progress!  I was able to reproduce a large failure and narrow it down to 3 particles.  From there, I was able to find a bug in the sse42 kernel.
In the custom binning version, if a pair separation falls outside the bins, we mark this by setting a bit mask and setting separation to rmax.  This happens here: https://github.com/adematti/Corrfunc/blob/eb3424a699771c077be900c78b1f7f7b8ead86d3/theory/DD/countpairs_kernels.c.src#L817-L825
But then in linear binning, we don't look at the bit mask, and just look at the separations.  This causes the rmax separation to potentially fall in the last bin, depending on the exact floating point value of the inverse separation.  This happens here: https://github.com/adematti/Corrfunc/blob/eb3424a699771c077be900c78b1f7f7b8ead86d3/theory/DD/countpairs_kernels.c.src#L835-L838
I admit I'm not sure I'm following the SSE code correctly.  It's not immediately obvious why we both set the bit mask and set the separation to rmax.  @manodeep, do you know why?
I'm also not sure if this is @manodeep's issue. It's definitely a bug (the reproducer is below), but it doesn't seem to depend on the compiler version, and it happens for SSE, not fallback.  But I haven't yet checked if the fallback/AVX/AVX512 kernels use the same pattern.

#!/usr/bin/env python3

import Corrfunc
import numpy as np
from astropy.table import Table

pos = np.array(Corrfunc.io.read_catalog())
pos = pos.T

#p = pos[:10]
p = pos[[7,8,9]]
print(p)

for i in range(len(p)):
    for j in range(i+1,len(p)):
        dist = ((p[i] - p[j])**2).sum(axis=-1)**0.5
        print(f'{i},{j} {dist}')

nbin = 7
bins = np.linspace(0.1, 7, nbin+1)
isa = 'sse42'

res = Corrfunc.theory.DD(1, 1, bins, *p.T, boxsize=420., isa=isa, verbose=False, bin_type='custom')
res = Table(res)

reslin = Corrfunc.theory.DD(1, 1, bins, *p.T, boxsize=420., isa=isa, verbose=False, bin_type='lin')
reslin = Table(reslin)

print(res)
print(reslin)

(forge) lgarrison@worker1012:~/abacus-analysis/Corrfunc/gh258$ ./repro.py
[[ 8.06210041  0.4235      4.89410019]
 [11.9282999   4.38660002  4.54409981]
 [11.95542526  4.32622337  4.51485252]]
0,1 5.547626567065066
0,2 5.5256725303853615
1,2 0.07236385736491963
       rmin               rmax        ravg npairs weightavg
------------------ ------------------ ---- ------ ---------
               0.1 1.0857142857142859  0.0      0       0.0
1.0857142857142859 2.0714285714285716  0.0      0       0.0
2.0714285714285716 3.0571428571428574  0.0      0       0.0
3.0571428571428574  4.042857142857143  0.0      0       0.0
 4.042857142857143 5.0285714285714285  0.0      0       0.0
5.0285714285714285  6.014285714285714  0.0      4       0.0
 6.014285714285714                7.0  0.0      0       0.0
       rmin               rmax        ravg npairs weightavg
------------------ ------------------ ---- ------ ---------
               0.1 1.0857142857142859  0.0      0       0.0
1.0857142857142859 2.0714285714285716  0.0      0       0.0
2.0714285714285716 3.0571428571428574  0.0      0       0.0
3.0571428571428574  4.042857142857143  0.0      0       0.0
 4.042857142857143 5.0285714285714285  0.0      0       0.0
5.0285714285714285  6.014285714285714  0.0      4       0.0
 6.014285714285714                7.0  0.0      2       0.0  # <<< this pair should not be here

Thanks to the excellent local scientific computing staff, I was able to get a gcc/9.2 binutils/2.33.1 toolchain to test with. Unfortunately, it did not exhibit the bug.
One suggestion that came up in conversation with them: gcc might be emitting some SIMD instructions even without -march=native. Can you try -march=x86-64 or something equally innocuous?

Was this on a CPU with AVX512F?

The new tests do fail on the CI, but only by 4 pairs. I think this is an "ordinary" failure due to floating-point precision, and not the gcc/9.2 failure, which was by thousands of pairs. When you run these tests locally, how many pairs do they fail by?

Yup - agreed. This is likely a floating-point round-off issue rather than a more severe underlying bug. I was interested to see if the CI (on different hardware + software infra)  failed on these new tests. Now that it looks like the CI fails, I will let the tests run through and see how many of the tests fail and by how much in each of the bins (i.e., are these floating point issues or something else)

Thanks to the excellent local scientific computing staff, I was able to get a gcc/9.2 binutils/2.33.1 toolchain to test with. Unfortunately, it did not exhibit the bug.
One suggestion that came up in conversation with them: gcc might be emitting some SIMD instructions even without -march=native. Can you try -march=x86-64 or something equally innocuous?

Was this on a CPU with AVX512F?

Yes, I tried multiple CPUs, including those with and without AVX512.

Thanks to the excellent local scientific computing staff, I was able to get a gcc/9.2 binutils/2.33.1 toolchain to test with. Unfortunately, it did not exhibit the bug.
One suggestion that came up in conversation with them: gcc might be emitting some SIMD instructions even without -march=native. Can you try -march=x86-64 or something equally innocuous?

Was this on a CPU with AVX512F?

Yes, I tried multiple CPUs, including those with and without AVX512.

Huh - I am using gcc/9.2.0 and binutils/2.33.1. Wonder why we are seeing different results on effectively the same CPU + toolchain.

Yeah, it's weird, but also possible I messed up something in my tests, or that the failure is coming in elsewhere, like a linked library.
Any thoughts on if my read of the SSE bug is correct? And what the best way to fix it might be?

I am still puzzled by the failure in the FALLBACK kernel, and the mis-match between the python results (as in the actual number of pairs, not just between the custom and linear bin types within a testing harness) and the C results.

output from the FAILING python tests (using all particles, not just the final 1000)

Done reading the data - time taken =        1.2 seconds
Beginning Theory Correlation functions calculations
binfile = [ 5.1  6.1  7.1  8.1  9.1 10.1 11.1 12.1 13.1 14.1 15.1 16.1 17.1 18.1
 19.1 20.1 21.1 22.1 23.1 24.1 25.1 26.1 27.1 28.1 29.1 30.1]
Running 3-D correlation function DD(r) (isa=fallback)
Custom binning
Running with points in [xmin,xmax] = 0.000400,419.999817 with periodic wrapping = 420.000000
Running with points in [ymin,ymax] = 0.000100,419.999512 with periodic wrapping = 420.000000
Running with points in [zmin,zmax] = 0.000300,419.999207 with periodic wrapping = 420.000000
In gridlink_double> Running with [nmesh_x, nmesh_y, nmesh_z]  = 27,27,13.  Time taken =   0.164 sec
Using fallback kernel
0%.........10%.........20%.........30%.........40%.........50%.........60%.........70%.........80%.........90%.........100% done. Time taken = 44.157 secs
Linear binning
Running with points in [xmin,xmax] = 0.000400,419.999817 with periodic wrapping = 420.000000
Running with points in [ymin,ymax] = 0.000100,419.999512 with periodic wrapping = 420.000000
Running with points in [zmin,zmax] = 0.000300,419.999207 with periodic wrapping = 420.000000
In gridlink_double> Running with [nmesh_x, nmesh_y, nmesh_z]  = 27,27,13.  Time taken =   0.148 sec
0%.........10%.........20%.........30%.........40%.........50%.........60%.........70%.........80%.........90%.........100% done. Time taken = 26.584 secs
Mis-match for npairs: (a, b) = [(62877472, 62754784), (78053816, 77985808), (95087768, 95001016), (113898272, 113848184), (134886224, 134795296), (157769872, 157678056), (181821432, 181754832), (207652120, 207602624), (235347640, 235250760), (264986328, 264816112), (296809304, 296635800), (330338968, 330253816), (365333792, 365246240), (403218408, 403068456), (442352288, 442215208), (483228064, 483199000), (526676640, 526695184), (571982752, 572006992), (619704272, 619652040), (669305888, 669225568), (721172136, 721062376), (774778408, 774635120), (830188704, 830089960), (888092192, 887990376), (946930304, 946695208)]
Mis-match for weightavg: (a, b) = [(0.562825815712476, 0.5628193730680126), (0.5627846481782354, 0.5627211027502464), (0.5624910928362716, 0.5624627003045992), (0.5625675596678956, 0.5625419184184755), (0.5626204591622173, 0.5625463179074152), (0.5626152639512979, 0.5625515815364374), (0.5624795476034595, 0.5624025335691824), (0.5625979690855286, 0.5625576307882932), (0.5625224962095966, 0.5624903745251197), (0.5626959192348553, 0.5626562965625453), (0.5626557659138799, 0.5626197098970304), (0.5627131464760827, 0.5626851580675043), (0.5625921173936103, 0.5625494022708031), (0.562640937769052, 0.5626023279874809), (0.5626230714190515, 0.5625917733991326), (0.5626902686220372, 0.562672483557968), (0.5626678796933742, 0.5626525397298503), (0.5626999385007042, 0.5626957858102613), (0.5626380544624989, 0.5626038471708693), (0.5626336402866137, 0.5626080653046808), (0.5626252511128291, 0.5625942514881577), (0.5626175708534336, 0.5625871978209532), (0.5625824122697372, 0.5625531941158517), (0.5626055249865471, 0.5625807147083433), (0.5626742075638334, 0.5626401289302878)]
Traceback (most recent call last):
  File "/apps/skylake/software/Python/3.8.5-gni-2020.0/lib/python3.8/runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/apps/skylake/software/Python/3.8.5-gni-2020.0/lib/python3.8/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/msinha/research/JournalSubmissions/SpeedingUpCode/Corrfunc/tests.py", line 184, in <module>
    test_linear_binning_theory(isa=isa)
  File "/home/msinha/research/JournalSubmissions/SpeedingUpCode/Corrfunc/tests.py", line 147, in test_linear_binning_theory
    test_bin_type(DD, autocorr, nthreads, binfile, x, y, z, weights1=w,
  File "/home/msinha/research/JournalSubmissions/SpeedingUpCode/Corrfunc/tests.py", line 143, in test_bin_type
    assert allclose(*res)
AssertionError

output from the PASSING C tests (I am printing out npairs even when the number of pairs match)

[rmin, rmax, nbins] = [5.10, 30.10, 25], isa =   FALLBACK Custom binning
Running with points in [xmin,xmax] = 0.000400,419.999817 with periodic wrapping = 420.000000
Running with points in [ymin,ymax] = 0.000100,419.999512 with periodic wrapping = 420.000000
Running with points in [zmin,zmax] = 0.000300,419.999207 with periodic wrapping = 420.000000
In gridlink_double> Running with [nmesh_x, nmesh_y, nmesh_z]  = 27,27,13.  Time taken =   0.231 sec
Using fallback kernel
0%.........10%.........20%.........30%.........40%.........50%.........60%.........70%.........80%.........90%.........100% done. Time taken = 12.521 secs
Linear binning
Running with points in [xmin,xmax] = 0.000400,419.999817 with periodic wrapping = 420.000000
Running with points in [ymin,ymax] = 0.000100,419.999512 with periodic wrapping = 420.000000
Running with points in [zmin,zmax] = 0.000300,419.999207 with periodic wrapping = 420.000000
In gridlink_double> Running with [nmesh_x, nmesh_y, nmesh_z]  = 27,27,13.  Time taken =   0.155 sec
0%.........10%.........20%.........30%.........40%.........50%.........60%.........70%.........80%.........90%.........100% done. Time taken =  7.410 secs
[nbin = 0] reference npairs = 15719370 computed npairs = 15719370
[nbin = 1] reference npairs = 19513442 computed npairs = 19513442
[nbin = 2] reference npairs = 23771964 computed npairs = 23771964
[nbin = 3] reference npairs = 28474552 computed npairs = 28474552
[nbin = 4] reference npairs = 33721542 computed npairs = 33721542
[nbin = 5] reference npairs = 39442494 computed npairs = 39442494
[nbin = 6] reference npairs = 45455368 computed npairs = 45455368
[nbin = 7] reference npairs = 51913026 computed npairs = 51913026
[nbin = 8] reference npairs = 58836836 computed npairs = 58836836
[nbin = 9] reference npairs = 66246674 computed npairs = 66246674
[nbin = 10] reference npairs = 74202316 computed npairs = 74202316
[nbin = 11] reference npairs = 82584722 computed npairs = 82584722
[nbin = 12] reference npairs = 91333442 computed npairs = 91333442
[nbin = 13] reference npairs = 100804634 computed npairs = 100804634
[nbin = 14] reference npairs = 110588016 computed npairs = 110588016
[nbin = 15] reference npairs = 120807052 computed npairs = 120807052
[nbin = 16] reference npairs = 131669182 computed npairs = 131669182
[nbin = 17] reference npairs = 142995722 computed npairs = 142995722
[nbin = 18] reference npairs = 154926102 computed npairs = 154926102
[nbin = 19] reference npairs = 167326390 computed npairs = 167326390
[nbin = 20] reference npairs = 180293032 computed npairs = 180293032
[nbin = 21] reference npairs = 193694562 computed npairs = 193694562
[nbin = 22] reference npairs = 207547282 computed npairs = 207547282
[nbin = 23] reference npairs = 222022944 computed npairs = 222022944
[nbin = 24] reference npairs = 236732684 computed npairs = 236732684
ESC[32mPASSEDESC[0m
ESC[0mntests run = 1 nfailed = 0ESC[0m

I have checked that the bins are identical between C and python.
(A related question is why does the python interface take so much longer? When running the python tests, the output ceases for quite a while after reaching 100%. And looks like the number of python- pairs is 4x the number of C pairs - which is suspiciously close to the number of threads)

Okay, I've made some progress! I was able to reproduce a large failure and narrow it down to 3 particles. From there, I was able to find a bug in the sse42 kernel.
In the custom binning version, if a pair separation falls outside the bins, we mark this by setting a bit mask and setting separation to rmax. This happens here: https://github.com/adematti/Corrfunc/blob/eb3424a699771c077be900c78b1f7f7b8ead86d3/theory/DD/countpairs_kernels.c.src#L817-L825
But then in linear binning, we don't look at the bit mask, and just look at the separations. This causes the rmax separation to potentially fall in the last bin, depending on the exact floating point value of the inverse separation. This happens here: https://github.com/adematti/Corrfunc/blob/eb3424a699771c077be900c78b1f7f7b8ead86d3/theory/DD/countpairs_kernels.c.src#L835-L838

To keep the same pattern as the other kernels, the computed bins will have to be blended with 0 for the bins where the bin_mask is set. bin 0 is for separations < rmin or separations >= rmax. Adding something like m_rpbin = SSE_BLEND_FLOATS_WITH_MASK(m_zero, m_rpbin, m_mask_left); after line 836 should do the trick.

I admit I'm not sure I'm following the SSE code correctly. It's not immediately obvious why we both set the bit mask and set the separation to rmax. @manodeep, do you know why?

From what I recall, I chose rmax because that way I would always have to make sure I did not accidentally have a <= rmax condition somewhere. (I am pedantic that bins are half-open -- lower limits are inclusive, and upper-limits are exclusive, and that behaviour should not change for the last bin)

I'm also not sure if this is @manodeep's issue. It's definitely a bug (the reproducer is below), but it doesn't seem to depend on the compiler version, and it happens for SSE, not fallback. But I haven't yet checked if the fallback/AVX/AVX512 kernels use the same pattern.

I suspect this is different.

I am still puzzled by the failure in the FALLBACK kernel, and the mis-match between the python results (as in the actual number of pairs, not just between the custom and linear bin types within a testing harness) and the C results.
(A related question is why does the python interface take so much longer? When running the python tests, the output ceases for quite a while after reaching 100%. And looks like the number of python- pairs is 4x the number of C pairs - which is suspiciously close to the number of threads)

Great catch. I think this strongly suggests this is an OpenMP library/linking issue. For example, the Corrfunc C extension is linked against one OpenMP library, but another OpenMP library gets loaded when a shared library is dlopen()'d by Python during a package import. Maybe this could happen via Numpy, especially if it is backed by a multi-threaded BLAS.  Or maybe via GSL (although that would just be a linking issue and not happen via dlopen()).  I'm not clear on exactly the conditions under which this will cause a problem, but maybe it's something of this flavor.
Similar issues were reported in #197.
I would suggest disabling OpenMP in common.mk then retesting; I bet that will fix the issue.  As to how to fix the issue with OpenMP enabled, I'm not sure what we can do from the Corrfunc side; it seems to depend on the compiler/other libraries. But let's confirm that it is an OpenMP issue first.

Yup - after disabling OPENMP and reinstalling the python extension, the python tests pass! Hooorraay!
This is very worrying! There are no tests on the python side - the assumption is that if the C tests are passing, then so should the python tests. We need to modify the call_correlation_function* codes into real tests. Think the actual calls are what the C tests are doing, we just need to add the comparisons with the known correct results.

@adematti is this something you could help with? The change is simply to modify this function (and the mocks one) to read in the correct results from the test directory (just like the bins) and assert that the results match.
This should be the last piece needed to close this PR.

Yes! I'll do that this weekend. Thank you so much for all the debugging!

Great -- thanks so much. @manodeep, can you confirm that these tests detect your issue? Then I think we are ready to merge.

Sorry to delay this PR further, but I would prefer to separate out the additional testing harness into a different PR. We don't have any tests for the python interface  - the call_correlation_functions were meant to show how to directly call the python extensions and were not intended to be actual tests. The python API is quite stable and it would be better to add in tests following the traditional python conventions (say pytests) and with filenames/function-names that conform to the usual conventions.
Related, we should deprecate and remove these call_correlation_functions scripts - these are no longer necessary and potentially provide the user with additional options that are not suitable for general use, perhaps as quickly as the next major release - i.e., v2.5.

Should have added - @adematti, please feel free to say that you would prefer someone else to take on the testing PR

An update on the race condition: switching to static OpenMP scheduling in countpairs_impl.c.src seems to reduce the severity of the issue - no longer 4x counts. Sadly, the tests still fail from python, and the number counts do not match what I see from C tests

Thanks, would be a nice bonus to get to the bottom of this issue! I can do the testing PR probably on Friday if you or @adematti don't have time to get to it before then. I think it won't be that much work now that the Python tests are hooked up to compare against the reference.

@lgarrison My knowledge of python is really not good enough to pull off the testing PR - you can almost certainly do that way faster than me!
One thing I would recommend is to keep the testing code intentionally very simple - the python testing setup in this PR is quite complex

Okay, there was a bug in the C tests that took a while to track down. The bins were being written to file with the %g specifier, which uses only 6 digits by default even for doubles. More precision is needed to match the linear binning. Switching to an exact representation with %a fixed the issue.
So now the C test pass (including the linear binning integration tests), as does test_lin.py.  I tried to support test_lin.py under Python 2.7, but the deeper I got, the more issues appeared. So while linear binning will likely run under Python 2.7, I think we'll just have to say the tests won't.
@manodeep, can you review test_lin.py and let me know if we are ready to merge?

Okay, there was a bug in the C tests that took a while to track down. The bins were being written to file with the %g specifier, which uses only 6 digits by default even for doubles. More precision is needed to match the linear binning. Switching to an exact representation with %a fixed the issue.

Awesome - makes sense!

So now the C test pass (including the linear binning integration tests), as does test_lin.py. I tried to support test_lin.py under Python 2.7, but the deeper I got, the more issues appeared. So while linear binning will likely run under Python 2.7, I think we'll just have to say the tests won't.

Do you have details about the issues you encountered? I am happy to try and take a look...
If we can't test with python2.7, what do you think about only supporting other bin types in >= python3? That way, we also signal that we are planning to drop python2 support

One problem was that Python 2.7 didn't like some of our test function signatures. I'm sure it's possible to fix, but the changes would touch all the test functions.
Sure, we could turn off linear binning just for Python 2.7, but it probably works fine and I tend to see all Python 2.7 usage these days as "use at your own risk" since it's been discontinued for so long!  We do have the warning of Python 2.7 Corrfunc deprecation in place, too.

Ohh good point about the deprecation warning - yup that is a reasonable warning that using python2.7 is in the "user-beware" territory.
Okay - I am going to look through the code in a little more detail. I remember having some thoughts about how the code could be structured, naming conventions etc

Sounds good! Let me know if I can help.

It is embarrassing how long this PR is taking me to review - just commenting to say that I am doing my best to get the review done

@adematti I am happy to review the remaining code - would that be useful?

@lgarrison Not sure if @adematti has notifications enabled or perhaps is slammed. How do you want to proceed on this PR? Seems like there might be a good use-case and the speedups are fairly reasonable from what I recall

Hi @manodeep, sorry for the delay (saw your message, then got extremely busy). I think this is up to you and Lehman! (the Corrfunc version we are using in DESI has evolved quite a lot).

I'm happy to merge this, it's a great feature as-is! @adematti, are there any bugs you found in the linear binning that might have been addressed in the DESI fork but not here?

yes, I was just looking at this. The only bug I remember of was: https://github.com/adematti/Corrfunc/blob/6ae2b9f4e470558ceb0b1d59fb0c2f62ff4bdbdc/mocks/DDtheta_mocks/countpairs_theta_mocks_kernels.c.src#L924
which should be:
if (need_rpavg || bin_type == BIN_LIN) {
(see https://github.com/adematti/Corrfunc/commit/8d95155dfb9da1f350cd8e318f6358e55f3a0018)

Ahh thanks @adematti :)
@lgarrison Do you want to merge this PR into a separate branch (we have one already right?) and then sort it out from there?

yes, I was just looking at this. The only bug I remember of was due to this line: https://github.com/adematti/Corrfunc/blob/6ae2b9f4e470558ceb0b1d59fb0c2f62ff4bdbdc/mocks/DDtheta_mocks/countpairs_theta_mocks_kernels.c.src#L924 which should be: if (need_rpavg || bin_type == BIN_LIN) { (see adematti@8d95155)

@adematti This fix s already in the PR right?

@manodeep I think this branch still needs to be fixed; I'll do that in the next day or two.

(I found myself a couple of minutes to fix the bug)

@manodeep Checks are passing; let's merge?

@lgarrison I was thinking of merging to a different branch - there are some naming conventions etc that I would like alter. Is the linearbinning-rounding appropriate or should we just create a new branch?

If you're feeling motivated to do another editing pass, then sure, let's merge to a new branch. But the PR has been open for 8 months, and I think functionally it's ready to go! It would be great to get this released so our users can experience the benefits.

@adematti I have merged this PR into the repo - many many thanks for such a massive effort!
@lgarrison Yeah I know - but now that I am back at ~full-time, the minor refinemennts should be much easier to get done

Hey @manodeep, have you had a chance to make these refinements? If not, I suggest we go ahead and merge this into master before the branches diverge even more.


Chore: Move fusion client code

Changelog Description
Moved fusion client code next to server codebase.
Testing notes:

Create package of fusion and core.
Upload both to server.
Make sure AYON launcher is using client code from this PR (e.g using dev paths).
Fusion integration should work as before.

@iLLiCiTiT do we still need the settings_category for most of these?

do we still need the settings_category for most of these?

Nevermind this - it seems there are no publish plug-in settings for Fusion currently and the Creator settings appear to work.

Side note:
It's weird that there apparently are no publisher plug-in settings for AYON Fusion even though OpenPype did seem to have them?

So separately from this PR we might want to expose that again.

Weird indeed, agree, different PR.


Switch to using keywords for syntax highlighting

Prompted by @roflcopter4's PR #276, explore using keywords for syntax highlighting.
Example: Replace
syn match perlStatementIPC "\v<%(msg%(ctl|get|rcv|snd)|sem%(ctl|get|op)|shm%(ctl|get|read|write))>"

with
syn keyword perlStatementIPC msgctl msgget msgrcv msgsnd semctl semget semop shmctl shmget shmread shmwrite

It's certainly easier to read. Is it faster? Can we measure?

I was a bit concerned in this sort of case. It's conceivable that regular expressions may save time for groups of keywords like this that are so similar. I'm not sure how you'd test this though. I've only really noticed the difference between match and keyword in a few canned cases that involved flooding vim with tens of thousands of things to check.

Thanks again for looking into this, @roflcopter4.  AFAIK the only reason for "correctness" that impacts the use of keyword vs match is that the fat comma operator won't quote its left-hand side in the case of keywords - for example:
Match (push is highlighted as a string):

Keyword (push is highlighted as a keyword):

As far as performance goes, I measured last year and saw a negligible difference - sadly, while I documented that, I didn't write down what version of Vim I was using, or what files I was testing against.  Out of all likelihood I was testing the highlighting of the perl5 corpus that the vim-perl tests run against.  If you're interested, I can dig up this information.
Regardless of the files I've tested against, I have a feeling the corpus doesn't contain some of the more pathological cases that people run into with vim-perl, so I wouldn't be suprised if keywords helped more in larger files!

I'm concerned about the fat comma issue. I think that having push => 3 highlighting "push" as a keyword would be a serious step backwards.

The best thing I can think of is to make it an option: some people might be willing to sacrifice the correct highlighting in favor of better speed.  I'd want to see some benchmarks that show substantial improvements in some slower-to-highlight files to justify that, though.
Another option, while it's very new and experimental, is to play with Vim's new text properties feature.  Even once that API stabilizes, we would probably have to support syn match-based keywords for a while, so it might not be worth the additional complexity.

I've run into this sort of issue before in my various code highlighting plugins. Every time it's reared its ugly head I have failed to solve it in any kind of satisfactory way. The only option that seems to work at all reliably is to use syntax matches instead of keywords. I didn't think this would crop up here. Looks like I forgot that this is Perl we're talking about.
I'm not even going to try to solve this. I've tried before, believe me. Maybe using a new feature like that could be possible (does Neovim support it too?) at some point but who knows. For now the smart thing is just to give up.

I think it's based on something that Neovim pioneered, but I don't track Neovim's development too closely.
Sorry to rain on your parade @roflcopter4 =( Thanks for giving it a shot!


[Usage]: How to inference model with multi-gpus

Your current environment
llm = LLM(model=model_name, max_model_len=8192, gpu_memory_utilization=0.8, tensor_parallel_size=8, dtype="bfloat16")
INFO 03-20 06:43:28 config.py:433] Custom all-reduce kernels are temporarily disabled due to stability issues. We will re-enable them once the issues are resolved.
2024-03-20 06:43:30,508	WARNING services.py:1996 -- WARNING: The object store is using /tmp instead of /dev/shm because /dev/shm has only 62910464 bytes available. This will harm performance! You may be able to free up space by deleting files in /dev/shm. If you are inside a Docker container, you can increase /dev/shm size by passing '--shm-size=10.24gb' to 'docker run' (or add it to the run_options list in a Ray cluster config). Make sure to set this to more than 30% of available RAM.
2024-03-20 06:43:31,684	INFO worker.py:1724 -- Started a local Ray instance.
INFO 03-20 06:43:33 llm_engine.py:87] Initializing an LLM engine with config: model='deepseek-ai/deepseek-coder-33b-base', tokenizer='deepseek-ai/deepseek-coder-33b-base', tokenizer_mode=auto, revision=None, tokenizer_revision=None, trust_remote_code=False, dtype=torch.bfloat16, max_seq_len=8192, download_dir=None, load_format=auto, tensor_parallel_size=8, disable_custom_all_reduce=True, quantization=None, enforce_eager=False, kv_cache_dtype=auto, device_config=cuda, seed=0)
Special tokens have been added in the vocabulary, make sure the associated word embeddings are fine-tuned or trained.
RuntimeError: torch.distributed is already initialized but the torch world size does not match parallel_config.world_size (1 vs. 8)
Here is information about my local environment:
ubuntu 22.04
Driver Version: 535.161.07 CUDA Version: 12.2
8x RTX A6000 with 80GB on local machine
Python 3.8.10
transformers: 4.39.0
vllm: 0.3.3
How would you like to use vllm
I want to run inference of a DeepSeek-Coder-33b model with 8 gpus.  Before I tried DeepSeek-Coder-6.7b model, I could generate output. How to load model with multi-gpus?
outputs = llm.generate(prompts, sampling_params)
Print the outputs.
for output in outputs:
prompt = output.prompt
generated_text = output.outputs[0].text
print(f"Prompt: {prompt!r}, Generated text: {generated_text!r}")

It looks like you're using Docker and your container only has 1 GPU available to it.
Are you launching it with --gpus all?

I checked gpus 8 and launched with --gpus all

+1 Launching with gpu all but it's only using one gpu.
sudo docker run --runtime nvidia --gpus all  -v ~/.cache/huggingface:/root/.cache/huggingface  --env "HUGGING_FACE_HUB_TOKEN=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"  -p 8000:8000  --ipc=host  vllm/vllm-openai:latest  --model Qwen/Qwen1.5-72B-Chat-GPTQ-Int4 --quantization gptq  --max-model-len 10096

NVIDIA-SMI:
| NVIDIA-SMI 550.54.14              Driver Version: 550.54.14      CUDA Version: 12.4     |
|-----------------------------------------+------------------------+----------------------+
| GPU  Name                 Persistence-M | Bus-Id          Disp.A | Volatile Uncorr. ECC |
| Fan  Temp   Perf          Pwr:Usage/Cap |           Memory-Usage | GPU-Util  Compute M. |
|                                         |                        |               MIG M. |
|=========================================+========================+======================|
|   0  NVIDIA A100 80GB PCIe          Off |   00000001:00:00.0 Off |                    0 |
| N/A   35C    P0             51W /  300W |       0MiB /  81920MiB |      0%      Default |
|                                         |                        |             Disabled |
+-----------------------------------------+------------------------+----------------------+
|   1  NVIDIA A100 80GB PCIe          Off |   00000002:00:00.0 Off |                    0 |
| N/A   36C    P0             52W /  300W |       0MiB /  81920MiB |      0%      Default |
|                                         |                        |             Disabled |
+-----------------------------------------+------------------------+----------------------+
|   2  NVIDIA A100 80GB PCIe          Off |   00000003:00:00.0 Off |                    0 |
| N/A   36C    P0             53W /  300W |       0MiB /  81920MiB |      0%      Default |
|                                         |                        |             Disabled |
+-----------------------------------------+------------------------+----------------------+
|   3  NVIDIA A100 80GB PCIe          Off |   00000004:00:00.0 Off |                    0 |
| N/A   35C    P0             52W /  300W |       0MiB /  81920MiB |      0%      Default |
|                                         |                        |             Disabled |
+-----------------------------------------+------------------------+----------------------+

+-----------------------------------------------------------------------------------------+
| Processes:                                                                              |
|  GPU   GI   CI        PID   Type   Process name                              GPU Memory |
|        ID   ID                                                               Usage      |
|=========================================================================================|
|  No running processes found                                                             |
+-----------------------------------------------------------------------------------------+

@ckj18 What does it say when you do nvidia-smi in the terminal?

same request. when I use CUDA_VISIBLE_DEVICES=0,1 to load the model, but only the first one gpu working, any solution? thanks

@hyperbolic-c vLLM will only use multiple GPUs if you see --tensor-parallel-size >1, are you doing that?

@ckj18 have you resolved your issue?

Yeah I resolved. Thank you sir
2024년 4월 12일 (금) 오후 3:52, Harry Mellor @.***>님이 작성:

@ckj18 https://github.com/ckj18 have you resolved your issue?
—
Reply to this email directly, view it on GitHub
https://github.com/vllm-project/vllm/issues/3525#issuecomment-2051103475,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ALUOBSKNAZGMVAKAUFSQTVTY46AEDAVCNFSM6AAAAABE64SPP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJRGEYDGNBXGU
.
You are receiving this because you were mentioned.Message ID:
@.***>

yes, use  --tensor-parallel-size 2
e.g:
#!/usr/bin/env bash
python -m vllm.entrypoints.openai.api_server --model internlm/internlm2_5-7b-chat --served-model-name internlm2_5-7b-chat --trust-remote-code --port 8082 --tensor-parallel-size 2

Will  --tensor-parallel-size 4 divide the model on 4 GPUs or load replicas of model on 4 GPUs?
Because when I run this command it loads replica of the model on all GPUs:
python -m vllm.entrypoints.openai.api_server --model mistralai/Mixtral-8x7B-Instruct-v0.1 --trust-remote-code --device cuda --tensor-parallel-size 4 --gpu-memory-utilization 0.95 --swap-space 1 --dtype auto --api-key <openai-api-key>

It is not loading replicas, it is using 1 process per GPU.

@hmellor  Okay, So how to distribute model across all GPUs using vllm? Because this command don't works for me.

The command in your previous message will distribute the model across 4 GPUs

@hmellor I think it is actually making relicas because when I load the model using transformers 'device_map' = 'auto' . It takes around 8 GB on each GPU but when I run the above command it takes 22GB on each GPU.
Can you please tell why is that happening? I am using "--model meta-llama/Meta-Llama-3.1-8B-Instruct"

I guarantee it is not replicas.
vLLM statically allocates KV cache and compiles a cuda graphs executable, both of which use memory that transformers is not.

Okay Got it. Thanks! Is there any way I can reduce memory usage without quantization?
Also How can I make model load on multiple nodes with multiple GPUs and perform inferencing?


Feature(testing): Testing feature libraries in a nx monorepo

Is there an existing issue for this?

[X] I have searched the existing issues

Which Transloco package(s) will this feature affect?
Transloco, Scoped Libs
Is your feature request related to a problem? Please describe
When writing unit tests there is always console.warn about missing translations but they are not missing.
Describe the solution you'd like
Documentations on a dynamic DRY approach on how to test standalone components that reside in a structure outside the app.
example:
apps

app1
app2
libs
feature1
feature2

i18n translations are in the features folder
Describe alternatives you've considered
N/A

Additional context
No response
I would like to make a pull request for this feature
No

Feel free to reopen with reproduction/test repo.


C# Console

Hi, i have an issue. I am trying to run the following code on a command promt/console but i keep getting this error
Severity	Code	Description	Project	File	Line	Suppression State
Error	CS5001	Program does not contain a static 'Main' method suitable for an entry point	KCSBOTBROS	C:\Users\ribei\OneDrive\Ambiente de Trabalho\KCSBOTBROS\CSC	1	Active
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Text.RegularExpressions;
using System.Threading;
using KuCoinApi.Net;
namespace KCSBOTBROS
{
public class Program
{
static async void Main(string[] args)
{
Console.Title = "KCSBotBros";
IKuCoinDotNet kucoin = new KuCoinDotNet("1", "1", "1");
Console.ForegroundColor = ConsoleColor.White;
Console.Write("Input the asset you want to invest with:");
string asset = Console.ReadLine();
Console.Write("Input the amount you want to invest:");
string amount = Console.ReadLine();
Console.Write("Input the desired profit(%):");
string profit = Console.ReadLine();
Console.Write("Input symbol:");
string symbol = Console.ReadLine();
string pair = symbol + "-" + asset;
decimal price;
var market_order = await kucoin.PlaceMarketOrder(pair, KuCoinApi.Net.Entities.Side.BUY, 1);
var orderdetails = await kucoin.GetOrder(market_order);
price = orderdetails.DealFunds / orderdetails.DealSize;
Console.WriteLine("Deal Funds:" + orderdetails.DealFunds + "\n" + "Deal Size:" + orderdetails.DealSize + "\n" + "Deal Price:" + price);
}
}
}

Change your Main() return type from void to Task
`
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Text.RegularExpressions;
using System.Threading;
using KuCoinApi.Net;
namespace KCSBOTBROS
{
public class Program
{
static async Task Main(string[] args)
{
Console.Title = "KCSBotBros";
IKuCoinDotNet kucoin = new KuCoinDotNet("1", "1", "1");
Console.ForegroundColor = ConsoleColor.White;
Console.Write("Input the asset you want to invest with:");
string asset = Console.ReadLine();
Console.Write("Input the amount you want to invest:");
string amount = Console.ReadLine();
Console.Write("Input the desired profit(%):");
string profit = Console.ReadLine();
Console.Write("Input symbol:");
string symbol = Console.ReadLine();
string pair = symbol + "-" + asset;
decimal price;
var market_order = await kucoin.PlaceMarketOrder(pair, KuCoinApi.Net.Entities.Side.BUY, 1);
var orderdetails = await kucoin.GetOrder(market_order);
price = orderdetails.DealFunds / orderdetails.DealSize;
Console.WriteLine("Deal Funds:" + orderdetails.DealFunds + "\n" + "Deal Size:" + orderdetails.DealSize + "\n" + "Deal Price:" + price);
}
}
}
`

I changed and now i get this error
Severity	Code	Description	Project	File	Line	Suppression State
Error	CS0246	The type or namespace name 'Task' could not be found (are you missing a using directive or an assembly reference?)	KCSBOTBROS	C:\Users\ribei\OneDrive\Ambiente de Trabalho\KCSBOTBROS\Program.cs	14	Active

It's working now, i forgot to add using System.Threading.Tasks;
Thx for the help.
Also is it possible to use funds on the market order function instead of size?

?

?

It's working now, i forgot to add using System.Threading.Tasks;
Thx for the help.
Also is it possible to use funds on the market order function instead of size?

Check the MarketOrderParams object

It doesn't let me edit, it's locked! How can i change it and what i write?
A terça, 13/04/2021, 21:17, Scheetz @.***> escreveu:

Check the MarketOrderParams object
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/mscheetz/KuCoinApi.Net/issues/20#issuecomment-819024907,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ATOBM7IALMCZ2IULCCIDLYTTISRHDANCNFSM42X2EAPQ
.

?
A terça, 13/04/2021, 21:37, Diogo Ribeiro @.***>
escreveu:

It doesn't let me edit, it's locked! How can i change it and what i write?
A terça, 13/04/2021, 21:17, Scheetz @.***> escreveu:

Check the MarketOrderParams object
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/mscheetz/KuCoinApi.Net/issues/20#issuecomment-819024907,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ATOBM7IALMCZ2IULCCIDLYTTISRHDANCNFSM42X2EAPQ
.

var marketOrderParams = new KuCoinApi.Net.Entities.MarketOrderParams()
{
Funds = 1000,
Pair = "BTC-USDT"
 };
            var trade = await kucoin.PlaceMarketOrder(marketOrderParams);

I get an error saying Input string was not in a correct format.
A quarta, 14/04/2021, 00:09, Scheetz @.***> escreveu:

var marketOrderParams = new KuCoinApi.Net.Entities.MarketOrderParams()
{
Funds = 1000,
Pair = "BTC-USDT"
};
var trade = await kucoin.PlaceMarketOrder(marketOrderParams);
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/mscheetz/KuCoinApi.Net/issues/20#issuecomment-819108559,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ATOBM7N5KFP2CQXTVJWZMNDTITFJVANCNFSM42X2EAPQ
.

I get an error saying Input string was not in a correct format.

?

Check out this unit test: https://github.com/mscheetz/KuCoinApi.Net/blob/869334ac02446c30fe05a6e61115f0b48ee9bc98/KuCoinApi.Net/KuCoinApi.Net.Tests/Tests/KuCoinRepositoryTests.cs#L177

i get this error System.NullReferenceException: 'Object reference not set to an instance of an object.' on the line
var trade = await kucoin.PlaceMarketOrder(marketorder);
the code:
var marketorder = new KuCoinApi.Net.Entities.MarketOrderParams
{
Pair = pair,
Side = KuCoinApi.Net.Entities.Side.BUY,
Funds = Convert.ToDecimal(amount)
};
var trade = await kucoin.PlaceMarketOrder(marketorder);


Modify index.html

Hello,
How can I modify the public/index.html file to change the default placeholder to another?
Because I had modified it but it has no effect after reloading kibana.

Can you try this? - Download load the plugin zip, modify the index.html, zip it back and install the updated zip. Not sure what do you mean by change the default placeholder to another

It gives me that error:
Retrieving metadata from plugin archive
Error: end of central directory record signature not found
at /usr/share/kibana/node_modules/yauzl/index.js:179:14
at /usr/share/kibana/node_modules/yauzl/index.js:539:5
at /usr/share/kibana/node_modules/fd-slicer/index.js:32:7
at FSReqWrap.wrapper [as oncomplete] (fs.js:683:17)
Plugin installation was unsuccessful due to error "Error retrieving metadata from plugin archive"

@chuski92 I tried following steps and it worked:

Shutdown Kibana
Uninstall existing logtrail installation from Kibana using ./kibana-plugin remove logtrail
Download the plugin zip from github.com and extract
Extract the zip and make changes in CSS/index.html
Zip the contents back using zip -r logtrail-xxx.zip kibana from the extracted dir
Install the plugin using kibana-plugin install file:///<path to newly created logtrail-xxx.zip>
Start Kibana

Thank you for your help!
It gives me that error:
Attempting to transfer from file:///tmp/logtrail-master.zip
Transferring 310729 bytes....................
Transfer complete
Retrieving metadata from plugin archive
Plugin installation was unsuccessful due to error "No kibana plugins found in archive"

@chuski92 Looks like you downloaded the logtrail source from master branch for editing. You need to download the plugin release zip from https://github.com/sivasamyk/logtrail/releases (based on your kibana version) and follows the above steps.

It works!
Thank you very much!


Investigate MSBuild tasks to integrate code generators seamlessly

Objective is to cut down any files that service developers aren't going to care about, from the web service project template.
During build time an MSBuild custom task can generate Uplift cpp and h files from a hpp2Soap.dll assembly (with embedded template c++ files) to a temp dir, then add them to the list of files to compile / include.
Resource files should also be embedded without service developers being aware of them, and it would also b nice to generate the exports.def file and add this to the MSBuild link task if possible.

I've done this with a .NET MSBuild assembly with custom build tasks, which forms the foundation of the future code generation system.
Couldn't get exports.def automatically generated with this method, so just going to leave in the template project (this will need modifying by the project template).
Resources inserted using UpdateResource Win API


Use Guzzle 6

Guzzle 3 actually have a Symfony compenent dependencies that is not compatible with SF 3: https://packagist.org/packages/guzzle/guzzle
This produce issues on composer update with converalls integrated.
I would suggest you to switch on Guzzle 6: https://packagist.org/packages/guzzlehttp/guzzle

Here you go: https://github.com/satooshi/php-coveralls/blob/master/composer.json#L19
closing

Oh, didn't see the 2.0 work sorry.


proof of concept for "generate code using JSON schema" using datamodel-codegen

I was trying to understand the project "generate code using JSON schema" better and I stumbled upon this: https://pydantic-docs.helpmanual.io/datamodel_code_generator/
It will need some additional work (like addition of the custom validators, working on the inheritance of schemas, etc.) but I think it can solve the problem. Take a look at the generated scripts to see how it works. (I have pushed all the code generated by datamodel-codegen as of now. In the future, we can write scripts to change the behavior according to our needs.)
There is also an option that creates all the classes in one script. We can use that if required.
Note: This PR is not to be merged. it's just to see how the datamodel-codegen generates the code and to decide whether we can use it for this project or not.
@astrojuanlu

There is tool called json-schema-for-humans. We can use it for documentation. I have generated an example (using Polygon.json), take a look: https://czml-doc-demo.rushabhv.repl.co/

Impressive! What I had in mind was to do it in some sort of custom way and attrs. But the use of pydantic here is great. I see that attrs and pydantic are often compared ([1], [2]).
About the HTML documentation of the schema, I like it but let's keep it separate from our code generation stuff (after all, is generally useful for the whole CZML community, not only for ourselves). I am already familiar with json-schema-for-humans 💪🏽
@rushabh-v my only concern is that switching from attrs to pydantic will make it impossible to migrate in an incremental way. czml3 is a small library anyway, so probably this won't be a problem, but we should keep it in mind nevertheless.
I will try to give this a more thorough review by the end of the week.

Hi @astrojuanlu, thanks for taking a look.
Sure, I will keep those points in mind.

Hi @astrojuanlu, I have pushed a new commit where the inheritance is also taken care of. I also have started working on my proposal, please let me know your feedback so that I can incorporate that.

Hi @astrojuanlu, a reminder.
Thanks!

Hi @astrojuanlu, I have a plan for doing this in an incremental way. We can generate a dependency graph for JSON schemas and go one level at a time (1 PR per level of the graph).
That way nothing will break and the review process will become very easy.
This makes sense to me. WDYT?

Hi @astrojuanlu, I have a plan for doing this in an incremental way. We can generate a dependency graph for JSON schemas and go one level at a time (1 PR per level of the graph).
That way nothing will break and the review process will get easier.
This makes sense to me. WDYT?

It sounds great :) Unlikely that we merge this PR, but it was a great way to get the ball rolling. Reviewing your GSOC proposal now. Thanks a lot!

Sure, closing this PR.
Thanks for your review :)

Hi @astrojuanlu, hope you are doing well!
I generated the dependency graph today and sorted it topologically. The levels in the graph are as follows:
(Note: Schemas of each level depend only on the schemas of its former levels.)
{
    "Level 0": [
        "DeletableProperty.json",
        "InterpolatableProperty.json",
        "NodeTransformations.json",
        "Articulations.json",
        "Clock.json",
        "CustomProperties.json",
        "package.json",
        "Document.json"
    ],
    "Level 1": [
        "LabelStyle.json",
        "Integer.json",
        "Boolean.json",
        "Double.json",
        "HeightReference.json",
        "Color.json",
        "Uri.json",
        "Repeat.json",
        "LineCount.json",
        "LineThickness.json",
        "LineOffset.json",
        "StripeOrientation.json",
        "ShadowMode.json",
        "DistanceDisplayCondition.json",
        "Position.json",
        "Translation.json",
        "Rotation.json",
        "Scale.json",
        "NearFarScalar.json",
        "ColorBlendMode.json",
        "ClassificationType.json",
        "CustomProperty.json",
        "RectangleCoordinates.json",
        "BoxDimensions.json",
        "PositionList.json",
        "PositionListOfLists.json",
        "ArcType.json",
        "BoundingRectangle.json",
        "CornerType.json",
        "Shape.json",
        "EllipsoidRadii.json",
        "String.json",
        "Font.json",
        "BackgroundPadding.json",
        "PixelOffset.json",
        "EyeOffset.json",
        "HorizontalOrigin.json",
        "VerticalOrigin.json",
        "DoubleList.json",
        "Articulation.json",
        "Orientation.json",
        "ViewFrom.json",
        "AlignedAxis.json",
        "DirectionList.json",
        "Direction.json"
    ],
    "Level 2": [
        "SolidColorMaterial.json",
        "ImageMaterial.json",
        "GridMaterial.json",
        "StripeMaterial.json",
        "CheckerboardMaterial.json",
        "PolylineOutlineMaterial.json",
        "PolylineArrowMaterial.json",
        "PolylineDashMaterial.json",
        "PolylineGlowMaterial.json",
        "NodeTransformation.json",
        "Model.json",
        "Point.json",
        "Label.json",
        "Billboard.json",
        "Tileset.json"
    ],
    "Level 3": [
        "Material.json",
        "PolylineMaterial.json"
    ],
    "Level 4": [
        "Cylinder.json",
        "Path.json",
        "Rectangle.json",
        "Polygon.json",
        "Ellipse.json",
        "Corridor.json",
        "Box.json",
        "PolylineVolume.json",
        "Polyline.json",
        "Ellipsoid.json",
        "Wall.json"
    ],
    "Level 5": [
        "Packet.json"
    ]
}

So to complete the work incrementally, maybe we can divide these schemas into 4 stages (as I mentioned in my proposal) like,
Stage 1 - Level 0 schemas
stage 2 - Level 1
Stage 3 - Level 2
Stage 4 - Level 4, 5, and 6

Please let me know, WDYT?
Thanks!


Cool Snapchat Filter

Is your feature request related to a problem? Please describe.
Want to apply a "Cool Snapchat Filter" on an image.
Describe the solution you'd like
Using python libraries to Implement a "Cool Snapchat Filter". ( OpenCV and Matplotlib)
Output

Hi this is not required in this repo as this is already available


Word Embedding tune

Have you tried to use other word embedding either then Glove?I have tried to replace the current word embedding with other one but I got surprised with the performance that's been dropped to become round 47%

I make all the token in SST unique each one has row and column after with sentence it lies and generated for each of them a new word embedding
I made sure that 'token_seq_digital' has the right indices(I am pretty sure it points on the right one)
The dictionary used by the train, test and dev contains all the token as they become unique
emb_mat_token is now for all the new emb vector the equivalent vector for the dictionary token
The new word_embedding_length now is 900. Do you have a clue why the performance is dropped in this manner? I expected some enhancement or even the same results but not less.

Sorry for the late reply.
I didn't test any other embedding method because it's not the focus of this paper.
Did you try to shorten the embedding dim to reduce the trainable parameters?

Thank you for your answer.

What do you mean by trying to shorten the embedding dim?
Where you train the embedding I can't discover it from the code. I know that you fine-tune glove but can't get where you do that
finally and you can skip this if you think it's out of your scope but shall we fine-tune all the embedding or glove only is what we need to do
Between I know this drop was happening due to the dimensionality of the embedding because I have tested the same with Glove but in a way, I have made emb_mat contains all the token not only unique ones and yes the performance has dropped sharply
Thank you again Tao and I will  appreciate if I get a quick response for these questions

I was asking about where in the code we tune the word representations? And in general if I used another word embedding is it possible to not need this tuning?

Got u, I learned some basic knowledge about the transferring/pretrained embeddings like ELMo and CoVe within these days.
The other embeddings can be fine-tuned in this framework. In current glove setup, I create a tensorflow trainable variable which is initialized by the pretrained glove. The variable will be automatically fine-tuned by backpropagation.
Therefore, in your setup, just set the variables in word embedding layer or pertained LSTM as "trainable".

Thank you!


Lower rpc throughput with more channels

Actual behavior
Rpc Throughput drops 1/3 when the number of incoming channels is increased from 60 to 3000. Is it expected behavior?
Netty version
4.1.6
JVM version (e.g. java -version)
OpenJDK Runtime Environment (build 1.8.0_222-8u222-b10-1ubuntu1~16.04.1-b10)
OpenJDK 64-Bit Server VM (build 25.222-b10, mixed mode
OS version (e.g. uname -a)
Linux  4.15.0-65-generic #74~16.04.1-Ubuntu SMP  x86_64 x86_64 x86_64 GNU/Linux

There is no way to help without more infos. This also sounds more like a question as a bug so please use stack overflow and provide more details


Admin UI: Optimise event data empty states

[x] On hover: show "Add lecturer" in light grey, or "Add location" for location
[x] Adjust on long hover tooltip copy to "Click to add a lecturer for this event", "Click to add a location for this event"
[x] Remove outline

Generally we should refer to organisers as "lecturers" in this app I think.

Reassigning to @Coenego for review.

Coverage remained the same at 100.0% when pulling 7e61c72639f55d983ab3060fbe3dfe4ad2a328c7 on bp323:issue-244 into 6353ba60830b8ce7e43b2d834f81f9cf259dec70 on CUL-DigitalServices:master.

Coverage remained the same at 100.0% when pulling 77320ca4f50c533d0e50b790f4c86bfc0f5b61eb on bp323:issue-244 into 6353ba60830b8ce7e43b2d834f81f9cf259dec70 on CUL-DigitalServices:master.


Memory Issues / Timeouts Running Docker Images on AWS

Awesome project! Super helpful!
I'm just using the images right from docker hub for basic-amd64 and all-amd64.
I'm seeing some strange memory behavior when running as a docker container on AWS.  When the crawl is running the memory utilization steadily climbs. If I let it go, it ends up going to 99+% and it gets super slow (or just starts timing out), so what I've been doing it every hour or so just rebooting the container to clear out the memory, but thats not the best practice. Curious whats going on with the memory management under the hood.
My typical crawl params:
            "crawler_params": {
                "headless": True,
                "simulate_user": True,
                "page_timeout": 3000,
                "remove_overlay_elements": True,
                "magic": True,
                "override_navigator": True,
            },

Example: At the beginning my crawl is running and memory is steadying being consumed. When the container is idling (for a solid 10 hours) memory isn't be released. At the end I just rebooted it to clear it out.
This machine has 32GB RAM, which should be plenty.

Haven't had a chance yet to debug this locally myself.

@kylepatel Thanks for trying Crawl4AI! Apologies for the delayed response—I’ve been busy updating documentation. Let’s address your issue:

Optimization Recommendations:

Disable the GPU by passing --disable-gpu as a browser argument.
Reduce the viewport size to improve performance.

Crawl Usage:

Please share your implementation details, especially your approach to creating instances of the AsyncWebCrawler class.
Avoid creating a new browser instance for each URL, this significantly impacts performance. Instead:

Create one browser instance and open new pages or tabs within it.
For sequential crawling, use a session ID to reuse the same tab rather than opening multiple tabs or processes.

Next Steps:

Let me know how you’ve used Crawl4AI and share your code if possible. I can provide more specific guidance based on your setup.

I also share a coding snippet that explains the general approach. Let me know your thoughts.
"""
This example demonstrates optimal browser usage patterns in Crawl4AI:
1. Sequential crawling with session reuse
2. Parallel crawling with browser instance reuse
3. Performance optimization settings
"""

import asyncio
import os
from typing import List
from crawl4ai import AsyncWebCrawler, BrowserConfig, CrawlerRunConfig
from crawl4ai.content_filter_strategy import PruningContentFilter
from crawl4ai.markdown_generation_strategy import DefaultMarkdownGenerator

async def crawl_sequential(urls: List[str]):
    """
    Sequential crawling using session reuse - most efficient for moderate workloads
    """
    print("\n=== Sequential Crawling with Session Reuse ===")

    # Configure browser with optimized settings
    browser_config = BrowserConfig(
        headless=True,
        browser_args=[
            "--disable-gpu",              # Disable GPU acceleration
            "--disable-dev-shm-usage",    # Disable /dev/shm usage
            "--no-sandbox",               # Required for Docker
        ],
        viewport={'width': 800, 'height': 600}  # Smaller viewport for better performance
    )

    # Configure crawl settings
    crawl_config = CrawlerRunConfig(
        content_filter=PruningContentFilter(),
        markdown_generator=DefaultMarkdownGenerator(),
        screenshot=False  # Disable screenshots if not needed
    )

    # Create single crawler instance
    crawler = AsyncWebCrawler(config=browser_config)
    await crawler.start()

    try:
        session_id = "session1"  # Use same session for all URLs
        for url in urls:
            result = await crawler.arun(
                url=url,
                config=crawl_config,
                session_id=session_id  # Reuse same browser tab
            )
            if result.success:
                print(f"Successfully crawled {url}")
                print(f"Content length: {len(result.markdown_v2.raw_markdown)}")
    finally:
        await crawler.close()

async def crawl_parallel(urls: List[str], max_concurrent: int = 3):
    """
    Parallel crawling while reusing browser instance - best for large workloads
    """
    print("\n=== Parallel Crawling with Browser Reuse ===")

    browser_config = BrowserConfig(
        headless=True,
        browser_args=["--disable-gpu", "--disable-dev-shm-usage", "--no-sandbox"],
        viewport={'width': 800, 'height': 600}
    )

    crawl_config = CrawlerRunConfig(
        content_filter=PruningContentFilter(),
        markdown_generator=DefaultMarkdownGenerator(),
        screenshot=False
    )

    # Create single crawler instance for all parallel tasks
    crawler = AsyncWebCrawler(config=browser_config)
    await crawler.start()

    try:
        # Create tasks in batches to control concurrency
        for i in range(0, len(urls), max_concurrent):
            batch = urls[i:i + max_concurrent]
            tasks = []

            for j, url in enumerate(batch):
                session_id = f"parallel_session_{j}"  # Different session per concurrent task
                task = crawler.arun(
                    url=url,
                    config=crawl_config,
                    session_id=session_id
                )
                tasks.append(task)

            # Wait for batch to complete
            results = await asyncio.gather(*tasks, return_exceptions=True)

            # Process results
            for url, result in zip(batch, results):
                if isinstance(result, Exception):
                    print(f"Error crawling {url}: {str(result)}")
                elif result.success:
                    print(f"Successfully crawled {url}")
                    print(f"Content length: {len(result.markdown_v2.raw_markdown)}")
    finally:
        await crawler.close()

async def main():
    # Example URLs
    urls = [
        "https://example.com/page1",
        "https://example.com/page2",
        "https://example.com/page3",
        "https://example.com/page4",
    ]

    # Demo sequential crawling
    await crawl_sequential(urls)

    # Demo parallel crawling
    await crawl_parallel(urls, max_concurrent=2)

if __name__ == "__main__":
    asyncio.run(main())

Looking forward to your details so I can assist further!

Ah I see, I'm currently just posting to the docker instance you've shared via dockerhub. I brought up that instance in my ECS Cluster and I'm hitting it like this:
    def _get_headers(self) -> Dict[str, str]:
        if not CRAWL4AI_API_TOKEN:
            return {}
        return {"Authorization": f"Bearer {CRAWL4AI_API_TOKEN}"}

    def create_request(
        self,
        url: str,
        llm: bool = False,
        screenshot: bool = False,
        custom_extraction: Optional[Dict[str, Any]] = None,
    ) -> Dict[str, Any]:
        request = {
            "urls": url,
            "priority": 10,
            "screenshot": screenshot,
            "crawler_params": {
                "headless": True,
                "browser_type": "chromium",
                "simulate_user": True,
                "page_timeout": 3000,
                "remove_overlay_elements": True,
                "magic": True,
                "verbose": True,
                "log_console": True,
                "override_navigator": True,
                "use_managed_browser": False,
            },
            "extra": {
                "word_count_threshold": 5,
                "process_iframes": False,
                "bypass_cache": False,
                "only_text": True,
                "exclude_external_links": True,
                "delay_before_return_html": 2.0,
            },
        }

        if llm:
            request["extraction_config"] = custom_extraction or {
                "type": "llm",
                "params": {
                    "provider": "openai/gpt-4o-mini",
                    "instruction": """
                    Analyze and extract the following from the webpage content for a RAG system.
                    """,
                },
            }

        return request

    def crawl(
        self,
        url: str,
        llm: bool = False,
        screenshot: bool = False,
        max_wait: int = 60,
        custom_extraction: Optional[Dict[str, Any]] = None,
    ) -> Dict[str, Any]:
        """
        Crawl a URL and wait for results

        Args:
            url: URL to crawl
            llm: Whether to use LLM extraction
            screenshot: Whether to take a screenshot
            max_wait: Maximum seconds to wait for completion
            custom_extraction: Custom extraction config to override default LLM config

        Returns:
            Dict containing crawl results including markdown content, extracted content if LLM used,
            links found, and content chunks
        """
        try:
            start_time = time.time()
            logging.info(f"Starting crawl request for {url}")

            request_data = self.create_request(url, llm, screenshot, custom_extraction)

            response = self.session.post(
                f"{self.base_url}/crawl", headers=self.headers, json=request_data
            )
            if response.status_code != 200:
                raise ValueError(f"Failed to submit crawl task: {response.text}")
            task_data = response.json()
            task_id = task_data["task_id"]

            while time.time() - start_time < max_wait:
                status_response = self.session.get(
                    f"{self.base_url}/task/{task_id}", headers=self.headers
                )
                if status_response.status_code != 200:
                    raise ValueError(f"Failed to get task status: {status_response.text}")
                status_data = status_response.json()

                if status_data.get("status") == "completed" and "result" in status_data:
                    endtime = time.time()
                    duration = round(endtime - start_time, 2)
                    logging.info(f"Finished crawl request for {url} after {duration} seconds")

                    return status_data["result"]
                elif status_data.get("status") == "failed":
                    raise ValueError(f"Task failed: {status_data.get('error', 'Unknown error')}")

            raise TimeoutError(f"Timeout after {max_wait} seconds")

        except requests.exceptions.RequestException as e:
            raise ValueError(f"Network error: {str(e)}")

I'm not instantiating anything myself directly.


feat: add historical state regen

Motivation

see #5846

Description

Add a historical state regen module which can regenerate old states
Optional to avoid breaking unrelated tests / mocks
Uses a dedicated worker thread with a JobFnQueue inside
nodejs, state transition, job queue metrics wired up
PubkeyIndexMap persisted between runs for faster repeated use

TODO

[ ] pull out any necessary configurability to CLI level
[ ] tests
[ ] dashboard

Running into issues opening the db in the worker thread.
It seems the leveldb way of allowing concurrency is have the consumer pass the pointer to the db handle to another thread. That's not currently possible with the leveldb package as it currently exists.

Converting to draft until the multithreaded database access issue is resolved

Any update on the status of this PR? Are we blocked on it somewhere externally?

Any update on the status of this PR?

waiting on me to fix this in response to @matthewkeil 's review

Ready for another round of review

this PR implements the fundamental work for historical state regen on worker thread, however I'm not sure how likely it could be fully functional in production, a request could take so long given archiveStateEpochFrequency = 1024, it could be up to 1024 * 10s ~= 2.8h given ~10s to process 32 blocks of an epoch like what I saw in n-historical state testing
we could explore storing incremental state diff with 2 purposes:

save disc space
this historical state regen: apply some diffs until the checkpoint state, and reprocess blocks like in this PR

cc @nazarhussain @philknows

we could explore storing incremental state diff with 2 purposes:

That is already in progress and will be complete very soon.

:tada: This PR is included in v1.21.0 :tada: