Unit Test for fsspmdm

@FreddieWitherden : could you please help with a script that unit tests fsspmdm? Can this be done with extending https://github.com/libxsmm/libxsmm/blob/master/samples/pyfr/pyfr_driver_asp_reg.c or do you recommend something else? Can we remove https://github.com/libxsmm/libxsmm/blob/master/samples/pyfr/pyfr_gemm_cm.c and https://github.com/libxsmm/libxsmm/blob/master/samples/pyfr/pyfr_gemm_rm.c in LIBXSMM 2.0 or do you think they provide some value?

pyfr_driver_asp_reg.c is what I normally use for testing/benchmarking.  Running it through all of the sparse matrices in the directory with some reference sizes usually does the job.

can you please submit a PR with a script doing exactly this (I plan to add this to our CI); and I guess the pyfr_gemm_rm.c / pyfr_gemm_rm.c can be removed ?

No problem. Is a shell script okay?
And yes, the other files can be removed.
Regards, Freddie.

On 31 Jan 2022, at 20:50, Alexander Heinecke @.***> wrote:

can you please submit a PR with a script doing exactly this (I plan to add this to our CI); and I guess the pyfr_gemm_rm.c / pyfr_gemm_rm.c can be removed ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.

Sh script is perfect, we then will massage it for buildkite / "make test". Thanks!

Merged #591
@hfp : can you please have a look at this:
Updated. When running the examples I get a few failures. This is not entirely unexpected (some of the kernels are larger than the default code buffer size). What is perhaps more interesting is that we fail with a stack smashing attempt which implies somewhere we're not being as careful as we should be (I am unsure if this is in the sample driver or libxsmm itself, if it is the latter we should probably fix the issue ASAP).

Running under a debugger:
(gdb) run
Starting program: /home/freddie/Programming/libxsmm/samples/pyfr/pyfr_driver_asp_reg ./mats/p6/pri/m460-sp.mtx 48000 10
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
CSR matrix data structure we just read:
rows: 588, columns: 196, elements: 12096
LIBXSMM ERROR (libxsmm_x86_instruction_vec_compute_3reg_mask_sae_imm8): code generation ran out of buffer capacity (error #90002)!
*** stack smashing detected ***: terminated

Program received signal SIGABRT, Aborted.
0x00007ffff6d151ec in ?? () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff6d151ec in ?? () from /lib64/libc.so.6
#1  0x00007ffff6cc9972 in raise () from /lib64/libc.so.6
#2  0x00007ffff6cb4449 in abort () from /lib64/libc.so.6
#3  0x00007ffff6d09768 in ?? () from /lib64/libc.so.6
#4  0x00007ffff6da3932 in __fortify_fail () from /lib64/libc.so.6
#5  0x00007ffff6da3910 in __stack_chk_fail () from /lib64/libc.so.6
#6  0x0000555555576088 in libxsmm_build (request=0x7fffffffd140, regindex=131072, code=0x7fffffffd130)
at ./src/libxsmm_main.c:2264
#7  0x0000555555584a3d in libxsmm_create_spgemm_csr_areg_v2 (gemm_shape=..., gemm_flags=1044, prefetch_flags=0, max_N=48,
row_ptr=0x5555556e70d0, column_idx=0x555555723590, values=0x55555570bb80) at ./src/libxsmm_main.c:5217
#8  0x0000555555568459 in libxsmm_dfsspmdm_create (M=588, N=48, K=196, lda=196, ldb=48000, ldc=48000, alpha=1, beta=0,
c_is_nt=1, a_dense=0x7ffef5b7b080) at ./src/libxsmm_fsspmdm.c:174
#9  0x000055555555cb5b in main (argc=<optimized out>, argv=<optimized out>) at pyfr_driver_asp_reg.c:275

I am guessing one of the x86 ASM generation instructions is forgetting to check the buffer size before inserting its code, and this is what is smashing the stack.

Any idea what was causing the buffer to fill up for the dense kernels?  I do not think they should be more than a few KB.

Oh... I see them filling up for the sparse one and now the code is no longer crashing. Also your call trace indicates it's happening in the sparse case. I just tightened the condition now.
LIBXSMM_VERSION: master-1.17-2200 (25692312)
CLX/DP      TRY    JIT    STA    COL
0..13      0      0      0      0
14..23      0      0      0      0
24..64      2      2      0      0
Registry and code: 13 MB + 16 KB (gemm=2)
Uptime: 0.592459 s
./mats/p5/tet/m0-sp.mtx passed!
LIBXSMM-JIT-DUMP(ptr:file) 0x7fdb4f9c7000 : libxsmm_abi1_clx_f64_nn_8x56x168_48000_168_48000_a1_b0_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0_decompress_A0_spfactor1.mxm
LIBXSMM-JIT-DUMP(ptr:file) 0x7fdb4f9c5000 : libxsmm_abi1_clx_f64_nn_8x56x168_48000_168_48000_a1_b1_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0_decompress_A0_spfactor1.mxm

I also just redid the same gdb run as you
(gdb) r ./mats/p6/pri/m460-sp.mtx 48000 10
Starting program: /home/heinecke/Projects/LIBXSMM_workspace/libxsmm_github/samples/pyfr/pyfr_driver_asp_reg ./mats/p6/pri/m460-sp.mtx 48000 10
[...]
[New Thread 0x7fff9d5c1640 (LWP 1357187)]                                                                                [0/1803]
[New Thread 0x7fff9cdc0640 (LWP 1357188)]
[New Thread 0x7fff9c5bf640 (LWP 1357189)]
[New Thread 0x7fff9bdbe640 (LWP 1357190)]
[New Thread 0x7fff9b5bd640 (LWP 1357191)]
[New Thread 0x7fff9adbc640 (LWP 1357192)]
[New Thread 0x7fff9a5bb640 (LWP 1357193)]
[New Thread 0x7fff99dba640 (LWP 1357194)]
[New Thread 0x7fff995b9640 (LWP 1357195)]
[New Thread 0x7fff98db8640 (LWP 1357196)]
...done!
computing BLAS (A dense) solution...
   Would it be possible to get this test script run on the usual architectures as part of the automated tests?

Yes, this is what we will do next and I think @hfp will help with that.

The PyFR driver was revised and the test script parallelized. The tests are performed as part of the Mixed pipeline (public) using a randomized compiler for LIBXSMM's source. The LIBXSMM_TARGET is kept dynamic like CPUID based, i.e., later testing can be improved checking all possible targets (non-x86 based tests will be added later as well).
Unfortunately, code generation still runs out of buffer capacity for some instances and several cases seem to fail.

Running out of buffer capacity is expected for some cases.  However, we should always be able to return a kernel (since the code will fall back to a dense kernel which should never fail to JIT).  If there a cases giving incorrect answers, let me know the matrix/architecture and I'll take a look.

Will do. I will also tune the error output to only appear if not handled later (if possible). I will simply paste cases here that fail (if any).

The following three cases are failing (tested LIBXSMM_TARGET=avx|avx2|skx):
./pyfr_driver_asp_reg mats/p1/tri/m460-sp.mtx 48000 10
./pyfr_driver_asp_reg mats/p1/tet/m460-sp.mtx 48000 10
./pyfr_driver_asp_reg mats/p1/pri/m460-sp.mtx 48000 10

Beside of other validation errors, e.g. avx vs avx2 also produce different results for the sparse case.

I've been through and debugged the issue for these matrices.  The problem is that they all have rows of all zeros; hence why the discrepancy only appears for beta = 0 calculations.  Given this, no operations are sequenced for these rows (as there is nothing to do).  This is fine when beta = 1 (which is how these m460 matrices occur in production), but an issue when beta = 0.

Should be fixed.

@FreddieWitherden Thanks, this fixed the main issue with the m460-case! There is one more issue specific for LIBXSMM_TARGET=avx2 (aka LIBXSMM_TARGET=hsw):
LIBXSMM_TARGET=hsw ./pyfr_driver_asp_reg mats/p1/pri/m460-sp.mtx 48000 10

I tested LIBXSMM_TARGET=x86,  LIBXSMM_TARGET=wsm, LIBXSMM_TARGET=avx, and LIBXSMM_TARGET=skx and they validate.

One more note: the same case (from above) fails on ARM also (and the max.error is similar to the HSW/AVX2 case. I tested on pretty much generic ARM like Jetson AGX (non-SVE).

On my system LIBXSMM_TARGET=hsw and LIBXSMM_TARGET=avx2 both work.
LIBXSMM_TARGET="avx2" LIBXSMM_VERBOSE="-1" ./pyfr_driver_asp_reg  mats/p1/pri/m460-sp.mtx 48000 10
CSR matrix data structure we just read:
rows: 18, columns: 6, elements: 36
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd3ad376000 : libxsmm_hsw_f64_nn_18x4x6_0_48000_48000_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd3ad373000 : libxsmm_hsw_f64_nn_18x8x6_0_48000_48000_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd3ad36f000 : libxsmm_hsw_f64_nn_18x16x6_0_48000_48000_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd3ad36d000 : libxsmm_abi1_hsw_f64_nn_4x18x6_48000_6_48000_a1_b0_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0.mxm
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd3ad376000 : libxsmm_hsw_f64_nn_18x4x6_0_48000_48000_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd3ad373000 : libxsmm_hsw_f64_nn_18x8x6_0_48000_48000_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd3ad370000 : libxsmm_hsw_f64_nn_18x16x6_0_48000_48000_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7fd2a866c000 : libxsmm_abi1_hsw_f64_nn_4x18x6_48000_6_48000_a1_b1_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0.mxm
computing golden solution...
...done!
computing libxsmm (A sparse) solution...
...done!
computing BLAS (A dense) solution...
...done!
max error beta=0 (libxmm vs. gold): 0.000000 (0.000000 != 0.000000)
max error beta=1 (libxmm vs. gold): 0.000000 (0.000000 != 0.000000)
max error beta=0 (dense vs. gold): 0.000000 (0.000000 != 0.000000)
max error beta=1 (dense vs. gold): 0.000000 (-2.041985 != -2.041985)
time[s] LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 0.001379
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 2.506564 (sparse)
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 7.519692 (dense)
GB/s    LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 6.684171
time[s] LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 0.000077
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 45.168461 (sparse)
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 135.505383 (dense)
GB/s    LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 210.786151
time[s] BLAS    (RM, M=18, N=48000, K=6, beta=0): 0.000339
GFLOPS  BLAS    (RM, M=18, N=48000, K=6, beta=0): 30.609165
GB/s    BLAS    (RM, M=18, N=48000, K=6, beta=0): 47.614256
time[s] BLAS    (RM, M=18, N=48000, K=6, beta=1): 0.000105
GFLOPS  BLAS    (RM, M=18, N=48000, K=6, beta=1): 98.842761
GB/s    BLAS    (RM, M=18, N=48000, K=6, beta=1): 153.755406

LIBXSMM_VERSION: master-1.17-2477 (25692589)
HSW/DP      TRY    JIT    STA    COL
0..13      2      2      0      0
14..23      0      0      0      0
24..64      0      0      0      0
Registry and code: 13 MB + 16 KB (gemm=2)
Uptime: 0.143396 s

Plain avx does not, but that is not supported by the sparse register kernels.
On my AWS Graviton3 instance (but with mainline libxsmm so it only runs NEON) I also have no issues.
OMP_NUM_THREADS=1 LIBXSMM_VERBOSE=-1 ./pyfr_driver_asp_reg ./mats/p1/pri/m460-sp.mtx 48 1000
CSR matrix data structure we just read:
rows: 18, columns: 6, elements: 36
LIBXSMM WARNING: timer is maybe not cycle-accurate!
LIBXSMM-JIT-DUMP(ptr:file) 0xffff97272000 : libxsmm_aarch64_f64_nn_18x2x6_0_48_48_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0xffff9726f000 : libxsmm_aarch64_f64_nn_18x4x6_0_48_48_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0xffff9723d000 : libxsmm_aarch64_f64_nn_18x8x6_0_48_48_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0xffff9726b000 : libxsmm_abi1_aarch64_f64_nn_8x18x6_48_6_48_a1_b0_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0.mxm
Sparse n = 4 fastest!
LIBXSMM-JIT-DUMP(ptr:file) 0xffff97272000 : libxsmm_aarch64_f64_nn_18x2x6_0_48_48_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0xffff9726f000 : libxsmm_aarch64_f64_nn_18x4x6_0_48_48_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0xffff9723a000 : libxsmm_aarch64_f64_nn_18x8x6_0_48_48_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0xffff97238000 : libxsmm_abi1_aarch64_f64_nn_8x18x6_48_6_48_a1_b1_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0.mxm
Sparse n = 2 fastest!
computing golden solution...
...done!
computing libxsmm (A sparse) solution...
...done!
computing BLAS (A dense) solution...
...done!
max error beta=0 (libxmm vs. gold): 0.000000 (0.000000 != 0.000000)
max error beta=1 (libxmm vs. gold): 0.000000 (0.000000 != 0.000000)
max error beta=0 (dense vs. gold): 0.000000 (0.000000 != 0.000000)
max error beta=1 (dense vs. gold): 0.000000 (2.007295 != 2.007295)
time[s] LIBXSMM (RM, M=18, N=48, K=6, beta=0): 0.000001
GFLOPS  LIBXSMM (RM, M=18, N=48, K=6, beta=0): 4.822835 (sparse)
GFLOPS  LIBXSMM (RM, M=18, N=48, K=6, beta=0): 14.468504 (dense)
GB/s    LIBXSMM (RM, M=18, N=48, K=6, beta=0): 12.860893
time[s] LIBXSMM (RM, M=18, N=48, K=6, beta=1): 0.000001
GFLOPS  LIBXSMM (RM, M=18, N=48, K=6, beta=1): 5.151827 (sparse)
GFLOPS  LIBXSMM (RM, M=18, N=48, K=6, beta=1): 15.455481 (dense)
GB/s    LIBXSMM (RM, M=18, N=48, K=6, beta=1): 24.041859
time[s] BLAS    (RM, M=18, N=48, K=6, beta=0): 0.000001
GFLOPS  BLAS    (RM, M=18, N=48, K=6, beta=0): 12.725251
GB/s    BLAS    (RM, M=18, N=48, K=6, beta=0): 19.794835
time[s] BLAS    (RM, M=18, N=48, K=6, beta=1): 0.000001
GFLOPS  BLAS    (RM, M=18, N=48, K=6, beta=1): 13.863205
GB/s    BLAS    (RM, M=18, N=48, K=6, beta=1): 21.564986

LIBXSMM_VERSION: master-1.17-2476 (25692588)
AARCH64/DP    TRY    JIT    STA    COL
0..13      2      2      0      0
14..23      0      0      0      0
24..64      0      0      0      0
Registry and code: 13 MB + 16 KB (gemm=2)
Uptime: 0.005057 s

(The poor AVX performance is due to streaming stores being used for the beta = 0 case which limits performance somewhat.)

On my system LIBXSMM_TARGET=hsw and LIBXSMM_TARGET=avx2 both work.

Now I am worried about the issue not reproducible on your end. See below my output and max error beta=0 (libxmm vs. gold): 1.989674 (-2.048643 != -0.058969) in particular.
$ LIBXSMM_TARGET=avx2 LIBXSMM_VERBOSE=-1 OMP_NUM_THREADS=1 ./pyfr_driver_asp_reg  mats/p1/pri/m460-sp.mtx 48000 10
CSR matrix data structure we just read:
rows: 18, columns: 6, elements: 36
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbe0df000 : libxsmm_hsw_f64_nn_18x4x6_0_48000_48000_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbe0dc000 : libxsmm_hsw_f64_nn_18x8x6_0_48000_48000_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbb4dc000 : libxsmm_hsw_f64_nn_18x16x6_0_48000_48000_a1_b0_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbbc3b000 : libxsmm_abi1_hsw_f64_nn_4x18x6_48000_6_48000_a1_b0_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0.mxm
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbe0df000 : libxsmm_hsw_f64_nn_18x4x6_0_48000_48000_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbe0dc000 : libxsmm_hsw_f64_nn_18x8x6_0_48000_48000_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbb4d9000 : libxsmm_hsw_f64_nn_18x16x6_0_48000_48000_a1_b1_p0.sreg
LIBXSMM-JIT-DUMP(ptr:file) 0x7f0cbb4d7000 : libxsmm_abi1_hsw_f64_nn_4x18x6_48000_6_48000_a1_b1_p0_br0_uh0_si0_tc-abid_avnni0_bvnni0_cvnni0.mxm
computing golden solution...
...done!
computing libxsmm (A sparse) solution...
...done!
computing BLAS (A dense) solution...
...done!
max error beta=0 (libxmm vs. gold): 1.989674 (-2.048643 != -0.058969)
max error beta=1 (libxmm vs. gold): 0.000000 (2.883354 != 2.883354)
max error beta=0 (dense vs. gold): 0.000000 (0.000000 != 0.000000)
max error beta=1 (dense vs. gold): 0.000000 (2.292798 != 2.292798)
time[s] LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 0.001415
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 2.442234 (sparse)
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 7.326703 (dense)
GB/s    LIBXSMM (RM, M=18, N=48000, K=6, beta=0): 6.512624
time[s] LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 0.000285
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 12.121018 (sparse)
GFLOPS  LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 36.363055 (dense)
GB/s    LIBXSMM (RM, M=18, N=48000, K=6, beta=1): 56.564752
time[s] BLAS    (RM, M=18, N=48000, K=6, beta=0): 0.000757
GFLOPS  BLAS    (RM, M=18, N=48000, K=6, beta=0): 13.688143
GB/s    BLAS    (RM, M=18, N=48000, K=6, beta=0): 21.292667
time[s] BLAS    (RM, M=18, N=48000, K=6, beta=1): 0.000585
GFLOPS  BLAS    (RM, M=18, N=48000, K=6, beta=1): 17.736735
GB/s    BLAS    (RM, M=18, N=48000, K=6, beta=1): 27.590476

LIBXSMM_VERSION: master-1.17-2477 (25692589)
HSW/DP      TRY    JIT    STA    COL
   0..13      2      2      0      0
  14..23      0      0      0      0
  24..64      0      0      0      0
Registry and code: 13 MB + 16 KB (gemm=2)
Uptime: 0.122778 s

Can you confirm the kernel hash?
md5sum libxsmm_hsw_f64_nn_18x4x6_0_48000_48000_a1_b0_p0.sreg
5628369d3bd9b606923731a25001b878  libxsmm_hsw_f64_nn_18x4x6_0_48000_48000_a1_b0_p0.sreg
``

Took some fiddling but I got to the bottom of the issue and have submitted a PR.  The routine is a bit of a pain to test due to the benchmarking code, so the actual kernel we return can come from one of four code paths (three sparse, one dense).  Which one happens to be faster depends to an extent on the hardware it is run one (L2 cache sizes, uop cache size etc.)
We may want to have some flags/environment variables to force a certain path when debugging.

We may want to have some flags/environment variables to force a certain path when debugging.

Good point. I will do it.

It might be worth seeing if there is some value in a more generic kernel evaluation framework in libxsmm.  Currently fsspmdm does it by itself, although there are likely several other routines where we have several code generation strategies and no a priori means of determining which is best.  This could also also make testing easier (as it would be part of a more general mechanism).

@FreddieWitherden That's a good suggestion. If you have any suggestion in mind, let me know. If this materializes, I will focus on supporting auto-tuning of kernel parameters not only kernels as a whole. Any other input is also welcome. Though, this is not coming out of the blue, I have some framework like this already focusing on Intel GPU and OpenCL.

Another issue seems we generate aligned LD/STs. As far as I remember, we wanted to ban vmovap? in favor of vmovup? (simply to avoid accidents and to improve code safety):
LIBXSMM_FSSPMDM_HINT=4 LIBXSMM_TARGET=hsw ./pyfr_driver_asp_reg mats/p1/pri/m460-sp.mtx 48000 10

Perhaps this is a side-effect of LIBXSMM_GEMM_FLAG_ALIGN_C_NTS_HINT.

Yes, the aligned instructions should only come out in non-temporal form.  The test script currently asks for these in the beta = 0 case.

Yes, the aligned instructions should only come out in non-temporal form. The test script currently asks for these in the beta = 0 case.

I think our alignment is insufficient depending on leading dimension being a multiple of the SIMD-width or not.

Yes, we should probably be more strict in validating ldc when deciding if to go ahead with NT store instructions or not:
https://github.com/libxsmm/libxsmm/blob/master/src/generator_spgemm_csr_asparse_reg.c#L320

I've done some first pass of fixing/increasing code-safety by checking leading dimension in case of LIBXSMM_GEMM_FLAG_ALIGN_A, LIBXSMM_GEMM_FLAG_ALIGN_C, and in case of LIBXSMM_GEMM_FLAG_ALIGN_C_NTS_HINT (in particular) for all architectures and code-paths.
At the moment I am checking for whole matrices (assuming batch operation), but I realized this must be applied for rows/columns (instead of whole matrices).

Tests unveiled issues for almost all dense cases with LIBXSMM_TARGET=hsw (and LIBXSMM_TARGET=skx), e.g.:
LIBXSMM_TARGET=hsw LIBXSMM_FSSPMDM_HINT=4 ./pyfr_driver_asp_reg mats/p6/hex/m460-sp.mtx 48000 10

Tests unveiled issues for almost all dense cases with LIBXSMM_TARGET=hsw (and LIBXSMM_TARGET=skx), e.g.:
LIBXSMM_TARGET=hsw LIBXSMM_FSSPMDM_HINT=4 ./pyfr_driver_asp_reg mats/p6/hex/m460-sp.mtx 48000 10

Are the kernels failing to JIT or just giving the wrong answer?  The p6 m460 matrices are quite big, so I would not be surprised if even the dense routines are exceeding the buffer size.

Tests unveiled issues for almost all dense cases with LIBXSMM_TARGET=hsw (and LIBXSMM_TARGET=skx), e.g.:
LIBXSMM_TARGET=hsw LIBXSMM_FSSPMDM_HINT=4 ./pyfr_driver_asp_reg mats/p6/hex/m460-sp.mtx 48000 10

Are the kernels failing to JIT or just giving the wrong answer? The p6 m460 matrices are quite big, so I would not be surprised if even the dense routines are exceeding the buffer size.

Could be. Though (beside of this most recent issue), ARM stuff crashes even earlier while the x86 code-gen returns an error gracefully, i.e., ARM code gen continues to issue instructions even if size of code-buffer is already exhausted. No matter which target arch, this could be about nasty "calibration" in the backend; needs perhaps some trial/time.

Are there currently any failing cases on x86?

Are there currently any failing cases on x86?

Everything fixed and ok now (selecting an explicit code-path aka LIBXSMM_FSSPMDM_HINT). We can keep this still open or break-out some related tasks such as avoiding buffer-overrun during code-generation and deeper CI-testing on ARM.

With any luck after the ARM checks all of the tests should now be passing.  From a testing standpoint we probably want macOS with an M1, something with SVE, and something with NEON.

With any luck after the ARM checks all of the tests should now be passing.

I can invite you to our test suite if you are interested (Buildkite).

From a testing standpoint we probably want macOS with an M1,

Yes, we can. I am on it (as a side-task). It's just that our M1 is not fully integrated into our setup. I will get the system at least operational with CI. Full integration is some effort as macOS deviates from the "norm" (authentication, NFS setup, Slurm, etc.).

something with SVE, and something with NEON.

I can do this only manually at this point. Let's see if this can be improved.

Great! Is anything else needed on the test side to handle LIBXSMM_FSSPMDM_HINT?  Also, to speed up the tests we can possibly reduce the test problem size from 48k to 48 (this also more closely matches what we use in PyFR).

Is anything else needed on the test side to handle LIBXSMM_FSSPMDM_HINT?

The test is now refined to run through all cases (after your comment).

reduce the test problem size from 48k to 48

Let's get it pass with LIBXSMM_FSSPMDM_HINT=1..4 at least one time. We can then tune the tests.

Is anything else needed on the test side to handle LIBXSMM_FSSPMDM_HINT?

The test is now refined to run through all cases (after your comment).

reduce the test problem size from 48k to 48

Let's get it pass with LIBXSMM_FSSPMDM_HINT=1..4 at least one time. We can then tune the tests.

Sounds good.  I think for AVX-512 the HINT=4 case will be a no-op as the four wide kernels require N be divisible by 4*8 = 32 which our current OpenMP block size of 48 does not permit.
Is anything needed to also test the single precision kernels?  I think the driver is now generically typed.

I think the driver is now generically typed.

It is, but this can be improved. I may collect common code between F32 and F64 and perhaps add an interface to also support our type-enums in addition to function-names with type-prefix (s/d).

I think for AVX-512 the HINT=4 case will be a no-op

I will do it anyway but at least reduce N on ARM to speedup tests. Though, I now run HINTed tests for avx2 only.

Is anything needed to also test the single precision kernels?

It's tested on x86 already but I restructured a bit and introduced F32-test on ARM.

@FreddieWitherden That's a good suggestion. If you have any suggestion in mind, let me know. If this materializes, I will focus on supporting auto-tuning of kernel parameters not only kernels as a whole. Any other input is also welcome. Though, this is not coming out of the blue, I have some framework like this already focusing on Intel GPU and OpenCL.

Currently for fsspmdm side we have two kernels (sparse and dense) and one parameter (n blocking for sparse).  But, I would like to also change the block size for the dense kernels and also make the amount of m blocking a parameter for the sparse kernels.  At the moment there is quite a bit of code required for each new kernel type, although with an automated framework it could be just a case of adding kernels to an array and pressing go.

Is there any documentation for fsspmdm? Browsing through it looks like it assumes row major ordering, and vector length multiples of 16?

We have been working on a paper describing fsspmdm, which I can send you a draft of.
The routine does indeed only support row-major ordering.  This is because the it attempts to exploit sparsity in the A matrix.  In the row-major case sparsity in rows of A are trivial to exploit: simply skip over rows of B when doing the dot products with all loads remain full-width.  Conversely, in the column-major case a single load from B will bring in several rows of B.  However, for a particular dot product we may only need a small subset of the 8 or 16 entries we've loaded.  This is wasteful and difficult to handle efficiently.

Definitely interested in a draft, thanks! My naive handwritten implementation of a CSR x dense column major MM has pretty atrocious performance. I'm curious how much better you're performing than MKL's sparse MM.
Am I correct that there are hard requirements on the padding and dimension of B in fsspmdm today?

Yes, B must be padded such that the number of columns (or at least the leading dimension) is a multiple of the vector length.  Given the number of columns in B is proportional to the number of elements this is not a particularly stringent requirement.

Let me close this issue. For future discussion, please consider a new report. Though, this discussion is already split between here and #681.


markdown links work in FROM but not in WHERE

What happened?
Great plugin, small problem:
LIST FROM #someTag WHERE key = [[]]
does not work if using markdown links instead of wikilinks. this.file.link doesn’t help.
However LIST FROM #someTag AND [[]] will work using either wiki or markdown links, so it seems something is wrong with the WHERE filter being aware of internal markdown links.
Steps to reproduce
Under settings: Files & links set use [[wikilinks]] to false
LIST FROM #someTag WHERE key = [[]]
DQL
when using internal markdown links and not wikilinks this doesn't work
LIST FROM #someTag WHERE key = [[]]

but
LIST FROM #someTag AND [[]]

will work for wiki or markdown links
JS
No response
Dataview Version
0.4.26
Obsidian Version
0.14.2
OS
Windows

woops. moved it to github. thanks


Feature/logi to wcms poc

Briefly describe your changes
Checklist before requesting a review

[ ] My pull request was branched from and targets the test branch
[ ] I have performed a self-review of my code
[ ] I have manually tested all packages affected (bonus points for automations)
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] My changes generate no new warnings
[ ] I have checked to ensure there aren't other open pull requests for the same change

Did you test your feature in the following environments?

[ ] Standalone Component
[ ] Standalone Full Editor
[ ] CDC Internal Checks

I'm less familiar with the data fetching inside of dashboards, so I'll defer the final approval to @mpallansch . If there's anything we need to discuss further let me know!


Using --abort-on-error flag causes an error when YouTube channel has no videos uploaded

DO NOT REMOVE OR SKIP THE ISSUE TEMPLATE

[X] I understand that I will be blocked if I remove or skip any mandatory* field

Checklist

[X] I'm reporting a bug unrelated to a specific site
[X] I've verified that I'm running yt-dlp version 2022.11.11 (update instructions) or later (specify commit)
[X] I've checked that all provided URLs are playable in a browser with the same IP and same login details
[X] I've checked that all URLs and arguments with special characters are properly quoted or escaped
[X] I've searched the bugtracker for similar issues including closed ones. DO NOT post duplicates
[X] I've read the guidelines for opening an issue

Provide a description that is worded well enough to be understood
When attempting to download a YouTube channel with no uploads while using --abort-on-error, yt-dlp exits with an error, which is unexpected (one would reasonably expect it to not cause an error and just not download anything) and inconsistent behavior (when attempting to download a Rumble channel with no videos no such error occurs). At most I would expect a warning message, but certainly not a fatal error.
Provide verbose output that clearly demonstrates the problem

[X] Run your yt-dlp command with -vU flag added (yt-dlp -vU <your command line>)
[X] Copy the WHOLE output (starting with [debug] Command-line config) and insert it below

Complete Verbose Output
$ yt-dlp -vU --ignore-config --abort-on-error 'https://youtube.com/@cats'
[debug] Command-line config: ['-vU', '--ignore-config', '--abort-on-error', 'https://youtube.com/@cats']
[debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8
[debug] yt-dlp version 2022.11.11 [8b64402] (pip)
[debug] Python 3.11.1 (CPython x86_64 64bit) - Linux-6.0.2-x86_64-with (OpenSSL 3.0.7 1 Nov 2022)
[debug] exe versions: none
[debug] Optional libraries: Cryptodome-3.16.0, brotli-1.0.9, certifi-2022.12.07, mutagen-1.46.0, sqlite3-2.6.0, websockets-10.4
[debug] Proxy map: {}
[debug] Loaded 1723 extractors
[debug] Fetching release info: https://api.github.com/repos/yt-dlp/yt-dlp/releases/latest
Latest version: 2022.11.11, Current version: 2022.11.11
yt-dlp is up to date (2022.11.11)
[debug] [youtube:tab] Extracting URL: https://youtube.com/@cats
[youtube:tab] @cats: Downloading webpage
[debug] [youtube:tab] Selected tab: 'featured' (home), Requested tab: ''
[youtube:tab] Downloading all uploads of the channel. To download only the videos in a specific tab, pass the tab's URL
ERROR: [youtube:tab] @cats: This channel has no uploads
  File "/usr/local/lib/python3.11/site-packages/yt_dlp/extractor/common.py", line 674, in extract
    ie_result = self._real_extract(url)
                ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/yt_dlp/extractor/youtube.py", line 4285, in wrapper
    info_dict = func(self, url, smuggled_data)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/yt_dlp/extractor/youtube.py", line 6029, in _real_extract
    raise ExtractorError('This channel has no uploads', expected=True)

$ yt-dlp -vU --ignore-config --abort-on-error 'https://rumble.com/abcd'
[debug] Command-line config: ['-vU', '--ignore-config', '--abort-on-error', 'https://rumble.com/abcd']
[debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8
[debug] yt-dlp version 2022.11.11 [8b64402] (pip)
[debug] Python 3.11.1 (CPython x86_64 64bit) - Linux-6.0.2-x86_64-with (OpenSSL 3.0.7 1 Nov 2022)
[debug] exe versions: none
[debug] Optional libraries: Cryptodome-3.16.0, brotli-1.0.9, certifi-2022.12.07, mutagen-1.46.0, sqlite3-2.6.0, websockets-10.4
[debug] Proxy map: {}
[debug] Loaded 1723 extractors
[debug] Fetching release info: https://api.github.com/repos/yt-dlp/yt-dlp/releases/latest
Latest version: 2022.11.11, Current version: 2022.11.11
yt-dlp is up to date (2022.11.11)
[debug] [generic] Extracting URL: https://rumble.com/abcd
[generic] abcd: Downloading webpage
[redirect] Following redirect to https://rumble.com/c/c-114491
[debug] [RumbleChannel] Extracting URL: https://rumble.com/c/c-114491
[download] Downloading playlist: c-114491
[RumbleChannel] c-114491: Downloading page 1
[RumbleChannel] c-114491: Downloading page 2
[RumbleChannel] Playlist c-114491: Downloading 0 videos of 0
[download] Finished downloading playlist: c-114491

@coletdjnz Do you remember what was the reason we decided on error?

@coletdjnz Do you remember what was the reason we decided on error?

as the tabs do not exist.
I recall discussing the possibility of returning an empty playlist in this case.

I think this issue should be closed as of today, yt-dlp does throw this error : ERROR: [youtube:tab] @<channel>: This channel has no uploads
~/x ❯ yt-dlp -vU --ignore-config --abort-on-error 'https://youtube.com/@cats'                                                              ⬢
[debug] Command-line config: ['-vU', '--ignore-config', '--abort-on-error', 'https://youtube.com/@cats']
[debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8
[debug] yt-dlp version<EMAIL_ADDRESS>from yt-dlp/yt-dlp [f10589e34] (zip)
[debug] Python 3.11.6 (CPython x86_64 64bit) - Linux-6.6.7-zen1-1-zen-x86_64-with-glibc2.38 (OpenSSL 3.2.0 23 Nov 2023, glibc 2.38)
[debug] exe versions: ffmpeg 6.1 (setts), ffprobe 6.1, rtmpdump 2.4
[debug] Optional libraries: certifi-2023.11.17, requests-2.23.0, secretstorage-3.3.3, sqlite3-3.44.2, urllib3-1.25.11, websockets-11.0.3
[debug] Proxy map: {}
[debug] Request Handlers: urllib
[debug] Loaded 1798 extractors
[debug] Fetching release info: https://api.github.com/repos/yt-dlp/yt-dlp/releases/latest
Latest version<EMAIL_ADDRESS>from yt-dlp/yt-dlp
yt-dlp is up to date<EMAIL_ADDRESS>from yt-dlp/yt-dlp)
[youtube:tab] Extracting URL: https://youtube.com/@cats
[youtube:tab] @cats: Downloading webpage
[debug] [youtube:tab] Selected tab: 'featured' (home), Requested tab: ''
[youtube:tab] Downloading all uploads of the channel. To download only the videos in a specific tab, pass the tab's URL
[download] Downloading playlist: Cats
[youtube:tab] Playlist Cats: Downloading 0 items
[download] Finished downloading playlist: Cats

@bashonly sorry I meant to say that this issue has been solved, as I don't see that error

Was fixed in 044886c220620a7679109e92352890e18b6079e3. Forgot to close issue


Remove hard dependency on PyVistaQt

PyVistaQt is currently a hard requirement of this package. It should be opt-in as it is an incredibly heavy dependency stack and most users (well me at least) want to use this in Jupyter and avoid Qt

Yup, I totally agree.
I'm going to make it optional, and put the glue functionality in a geovistaqt package. This'll address this dependency issue for you.
Now that I've finally got the first release of geovista banked, I can start to iterate quite quickly with frequent, small releases.
I'll get to this ASAP for you @banesullivan. I'm already scheduling another patch release v0.1.3 for next week, so I can roll this change into that for you 👍

@all-contributors please add @banesullivan for bug


Updated link to documentation

Previous link gives a 404 error, so have updated to point to the correct address, https://twilio-labs.github.io/socless/.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

[x] I acknowledge that all my contributions will be made under the project's license.

Thanks for your contribution @teejayen !
Unfortunately, I'd already merged another PR that was addressing the same issue before seeing yours. Closing this as done


Properly referencing the app.css and app.js files

Updating the paths for js/app.js and css/app.css to to be referenced via asset() so the the paths are always correctly generated.

@mikhaill Tell me, and how can they not work correctly?

LGTM. The leading slash implice that Laravel is sitting at the domain root. It may not.


Adjust timeouts for Data Refresh wait_for_completion step

Fixes

Fixes #447 by @AetherUnbound
Description

The data_refresh DAGs are currently timing out after one hour of the wait_for_completion step, while we ping the ingestion server for the status of the data refresh. The problem was that the task was not in reschedule mode, so it would not free the slot after an unsuccessful ping. This caused it to timeout on its lower execution_timeout rather than the timeout. (Relevant docs)
This PR:

Sets the wait_for_completion task into reschedule mode. Now it will timeout only if a single attempt at poking takes longer than the execution_timeout of 1hr, or if no poke is successful before the timeout
Updates timeout to 3 days instead of the default 1 week. Looking for confirmation that this is a reasonable value; I think it should be based on past runs
Updates the poke_interval to 15 minutes instead of the default 1 minute, as the refresh is a long running task and we don't need to check that aggressively. This poke interval can be overridden by the same DATA_REFRESH_POKE_INTERVAL env var for testing purposes 😄

Testing Instructions

Prereq: To run a data refresh DAG, you'll need to have the data_refresh pool set up (or else Airflow won't schedule any tasks). This is done in the Airflow UI by going to Admin > Pools and clicking the ➕ icon to create a new pool.

Set Pool to data_refresh
Set Slots to 1
Save and you should be good to go!

Try running a data refresh locally on main, and observe it from the Tree view (http://localhost:9090/tree?dag_id=audio_data_refresh). Notice that the wait_for_completion step stays in the 'running' state until it resolves.
Now checkout the branch and try again. Notice that the wait_for_completion step now enters the up_for_reschedule state after the first poke fails!
Checklist

[ ] My pull request has a descriptive title (not a vague title like Update index.md).
[ ] My pull request targets the default branch of the repository (main) or a parent feature branch.
[ ] My commit messages follow best practices.
[ ] My code follows the established code style of the repository.
[ ] I added or updated tests for the changes I made (if applicable).
[ ] I added or updated documentation (if applicable).
[ ] I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

I am sure I'm missing some important step(s), but I don't know what...

@obulat Ah, I think you'll need to create the data_refresh pool! The data refresh DAGs run in that special pool, and Airflow won't run the tasks if it can't find it. In the UI in Airflow you can go to Admin > Pools, then create a new pool. Set Pool to data_refresh and Slots to 1. Sorry about that, I'll add that to the testing instructions!

Ah, you may need to perform some of the testing setup steps listed here to actually get the data refresh connection set up! :smile: https://github.com/WordPress/openverse-catalog/pull/397

Sorry I missed this -- @AetherUnbound is right, there were some setup steps for the data refresh I neglected to add. I've copied them into the testing instructions here as well, my apologies!


feat: frame parsing by spec, error and warning reports, debugger allowing to debug only selected spec

Change Summary
This PR:

[x] - splits Frame parsing by specification (farcaster, openframes)
[x] - add optional reports property to each Frame result which is present only if there are any errors/warnings detected for given specification (renamed from errors property
[x] - fixes falling back to farcaster data only if of:accepts:farcaster is present
[x] - adds toggle group to change which specification user is debugging in the debugger. Shows only errors/warnings/success for given specification.

All changes are breaking changes hence minor version bump. Without this change it would not be able to debug only for specific specification.
Also I found that all our templates are possibly incorrect since the of:accepts:farcaster tag is missing therefore produced frames are not producing of:* meta tags. This is caused by fact that we had to split the parsing and also that you have to explicitly set the accepts header. Perhaps there is better solution to this, for example providing the information that we support farcasterby default, that would also make all templates valid open frames.
Merge Checklist

[x] PR has a Changeset
[x] PR includes documentation if necessary
[x] PR updates the boilerplates if necessary

@stephancill are we good to merge?


make fork tests more resilient

Which accounts have certain ERC20 on mainnet constantly changes. Make tests that require initial funding of fork test accounts more resilient by utilising a list of possible erc20 bag addresses and pick the one with the most funds.

Thanks for super quick review @shahthepro !


Lock the scroll position when the modal is up

When it's not this happens:
https://app.usebubbles.com/q5ej4S1KZbGhmQzTjujxft/comments-on-girlsintech-org/
This can do it, it's super tiny in bundle size too.
https://www.npmjs.com/package/body-scroll-lock

Is this currently happening? I uploaded a fix with #72. I see on the video that was recorded before the PR was merged.

Ah yeah. Not on my phone but will double check some other browsers!

great on modern browsers it seems fine, closing!


Update Terra Staging Access instructions

https://ebi-ait.github.io/hca-ebi-dev-team/admin_setup/Setting-up-access-to-Terra-staging-area.html#setting-up-access-to-terra-staging-area

I believe this has been done as per ticket #405 https://app.zenhub.com/workspaces/operations-5fa2d8f2df78bb000f7fb2b5/issues/ebi-ait/hca-ebi-dev-team/405 so closing.


Separate Sphinx D Domain into a python project

Currently ddomain.py is a little "hack" to make D kinda work with Sphinx. It could easily be put into a separate python project and probably added to sphinx.contrib.
Does it really make sense to maintain two libraries in one repository, each with their own issues?

Done: https://github.com/BenjaminSchaaf/sphinxddoc


do_not_route_options should not call any before blocks

Right now if you have a before block declared, it will still get called.

https://github.com/ruby-grape/grape/pull/1119

This failing spec confirms the issue
it 'should not call before when do_not_route_options!' do
  subject.do_not_route_options!

  before_called = false
  subject.before { before_called = true }

  subject.post :hello do
    'Hello World'
  end

  get '/hello'

  expect(last_response.status).to eql 405
  expect(before_called).to be_falsey
end

Failure/Error: expect(before_called).to be_falsey
  expected: falsey value
       got: true

This is also happens on 405 Not Allowed routes generated by the API

:+1: @arempe93 Maybe you want to finish #1119?

I'm not sure we would want to fix this, as a number of people use before blocks to set custom headers, that would then not be included in 405's.


EmojiGifTextField controller returns empty text value if EmojiGifPickerIcon not opened

Hi,
I'm using EmojiGifPickerIcon with the option fromStack: true,
However I noticed that the text field doesn't capture the enter string in case If I didn't open the EmojiPickerstack firstly. once I do that, the controller starts to capture the text value
What could be the workaround in order to enforce EmojiGifTextField to behave normally (To capture the input value event if the user didn't open the Emoji window"

Hi,
Could you explain the problem in a little more detail and send a code example where the problem occurs?

@worldwidee
it's the same example mentioned in README. simply when you use EmojiGIFPickerIcon along with EmojiTextField. and in case if you started to type inside the textfield, the controller TextEditingController won't detect any update happens there. but it will once you press and open the picker windows then you switch back to the keyboard again. only in this case it will start to sense any typing and the controller will be usable


Advance buttons for attributes MIA

Replace
https://github.com/Plushtoast/dsa5-foundryVTT/blob/master/templates/actors/parts/characteristics-large.html
in your installation or wait for the next patch.


Broken connection to Neurodesktop

OS: Windows
Neurodesktop App Version: 1.3.0
The Neurodesk App and The JupyterLab interface worked as usual, but opening the Neurodesktop failed.
Expected Behaviour:
Clicking the Neurodesktop button starts the Apache Guacamole window, where either RDP or VNC desktop connection can be selected
Error:
Instead of Starting the Apache Guacamole window to select the connection, a error 400 (I think) window was shown with some java problem (unfortunately no screenshot).
Restarting the app didn't fix the problem.
Solution:
After "Open Developer Tools -> Application -> Clear site data", the connection worked again

Hi Korbinian.
Thanks for logging the bug.
If it was fixed by clearing site data, I guess it was due to multiple instances spinned up so I tried to reproduce it by doing so.
Does the error look like this?

From what I remember, when clicking on the Neurodesktop button in the Launcher, a new window opened. Instead of the Guacamole window it showed a blank white window with an error. Maybe the javascript error was the same, I unfortunately didn't create a screenshot.

I found the cause of what I can replicate as in the screenshot above.
Every time new window is opened on top of existing window, docker starts another container with the same name name and try to remove the existing one but failed since the container is still running.
And cache is not cleared every time we start new window.
Fixed it in this commit 737f7043d7995398485e6ef303c2e9f3c4c46094 and new release.
I'll close the issue for now but feel free to re-open if you got the error again.


Background color issue with solarized vim theme

I just installed this theme for hyper and there seems to be an issue when using vim and the solarized.vim theme, as the background color is incorrect and makes things illegible. See screenshot:

Is there something I am doing wrong?

This happens both in bash and zsh. For reference here are my OS details and such:
Hyper Version: <IP_ADDRESS>4
Hyper Plugins: hypertheme, hyper-solarized-dark, hyper-nobold, hyper-tabs-enhanced, hyperline
zsh Version: 5.3.1
macOS Version: Sierra 10.12.6
I am using the vim colorscheme found here, unmodified.
Thoughts?

It also happens with the solarized colorscheme found here.

@welnanick It seems to be a common issue in Hyper. There is a PR opened here - https://github.com/zeit/hyper/issues/155
I am using OceanicNext until I figure this out. It works pretty well.

Darn, well I have a workaround using solarized8-vim and setting termguicolors, but its not complete, some of my highlighting settings in my .vimrc are still not working. I hope hyper fixes it soon!

Closing this for now, since it's not really a theme issue.


JAVA-2249: Remove stripTrailingZeroBytes from OPPToken

Remove stripTrailingZeroBytes from OPPToken
See https://datastax-oss.atlassian.net/projects/JAVA/issues/JAVA-2249
Modifications:

Remove code to strip trailing zero bytes upon creation of OPPToken
Remove corresponding unit test
Changed the condition for padding bytes in the split logic. In the existing behavior, it looks like 4 bytes will always be added because the existing condition doesn't detect when start.equals(end). Previously this went unnoticed because the 4 zero bytes would be stripped off. This came up in OPPTokenFactoryTest.should_split_range_producing_empty_splits_near_ring_end()

Thank you @amandachow!


🏝 Taking a break

Taking a break once again, issue section closed from creating new issues. I'm focusing on the other tasks of the project and IRL stuff.
Reason for break: I'm only maintainer on this project and to avoid burnout, I don't want huge amount of stuff to moderate during this time.
Few things during the break:

Do not create issues!

Bypassing the notice when clicking "new issue" and making issue anyway will give you a ban and instant delete for the issue!

No icon requests / non-PR submissions via comment either to any of issues, PRs or anywhere else!

Warning for the first offense, second time equals ban.
Comments that aren't related to those are fine (comment them here on this issue).

PRs are allowed. See bottom comment.

Important update!
PRs are temporarily NOT ALLOWED (exception for #937, since it was opened before closure), until I have revisioned CONTRIBUTING guidelines about icon grid and other stuff.
Subscribe to this issue to know when PRs are allowed again.

Just FYI if you didn't know:
You can use GitHub interaction limits to achieve this.
By setting those to collaborators only, you can still work in the repo but anyone wanting to open an issue, comment or do anything that would require energy from you only sees a message box that that isn't possible at the current time.

Would you be willing to pass the project on to others?

Okay, I guess it's time to break the silence from me: I'm not sure and even if I decide this, I can't say when.
Few months after break started, I got too busy with stuff on school / personal life and finally burned out on this project, so it's unlikely that I continue it. It had over 4 year run, which is pretty long for most FOSS projects.
There are other parts outside of GitHub that also possibly need migration (eg. Notion databases) and I would also need volunteers that understand how to make icons with all the standards in mind.
But for now this project is on indefinite hiatus.

I will remove it someday but there's also release date under it, so it's not that big of the issue

On https://aegis-icons.github.io/ it is still specified that the icon pack is updated monthly. Would be good to remove that so that people don't spend time updating the icon pack over and over again with the same version.

Done.

Would you be interested to bring it back to life if there will be more contributors / maintainers? I'm happy to help.


All non-standard tasks should pass unaltered annotations

Tasks are more varied than I thought. If the task isn't of a recognized type, instead of yielding an error, it will just pass the unaltered annotation through. Surveys now fall into this category, as well as whatever Old Weather uses. This was the previous behavior and I figured it was better to be specific, but turns out there's more out there than I thought.
The exception is if an unsupported task (i.e. not drawing/single/multiple/text/dropdown) is inside of a combo task, it will still yield a "task cannot be exported" for the annotation.
Review checklist

[ ] First, the most important one: is this PR small enough that you can actually review it? Feel free to just reject a branch if the changes are hard to review due to the length of the diff.
[ ] If there are any migrations, will they the previous version of the app work correctly after they've been run (e.g. the don't remove columns still known about by ActiveRecord).
[ ] If anything changed with regards to the public API, are those changes also documented in the apiary.apib file?
[ ] Are all the changes covered by tests? Think about any possible edge cases that might be left untested.

LGTM pending specs. Get it out if it all passes.


Cannot find explanation/documentation for how to send a direct message to only one user in Bolt logic

Description
I want to send a message directly to a user, rather than responding to them in channel. I've searched everywhere for an explanation or example of this in bolt-js, but I am not seeing anything on the internet that explains this.
What type of issue is this? (place an x in one of the [ ])

[x] question
[x] documentation related

Requirements (place an x in each of the [ ])

[✓x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
[x] I've read and agree to the Code of Conduct.
[x] I've searched for any related issues and avoided creating a duplicate issue.

Hi @tsibiski - Happy to point you in the direction of the documentation that will help you get off the ground. The method you want in this case is:

chat.postMessage: Here's a nice bit of helpful documentation. If you provide the user's name instead of the channel id, a DM will be created for you.

As you can see in the documentation, in order for your app to be able to call this method, you will need an access token (xoxb) with the required scopes for the method.

Thank you. I appreciate the link.

@tsibiski - Of course. Hope that gets you unblocked for now and if anything else comes up, you know where to find us. Some other helpful places I will share for troubleshooting are: 👍

All api methods available to you. Some times your use case will require a couple of different methods to accomplish.
Bolt-JS developer documentation. Tips on using Bolt and specific Bolt conventions.
Bolt-JS example apps for different use cases (like OAuth)


[Material][TextBox] Make all property values on Setters from VisualStates accessible via ThemeResources

What would you like to be added:
ThemeResources for property values that are currently set directly to Setters in VisualStates.
https://github.com/unoplatform/Uno.Themes/blob/569a9c0c6e5b3a2295702c85c9b2cd59e2758572/src/library/Uno.Material/Styles/Controls/v2/TextBox.xaml#L679-L688
As you can see, the properties RootBorder.BorderThickness and RootBorder.Padding have no ThemeResources like the other Setters.
Why is this needed:
It would be very useful since this way you can easily customize the style referring to the key of the ThemeResource. It would not be convenient to modify the setters in any other way.
Anything else we need to know?
I am having this problem with the Style MaterialOutlinedTextBoxStyle.

Besides that, we have found that the behavior of the MinHeight property is not working correctly.
A default Material TextBox with the Style MaterialOutlinedTextBoxStyle has the following values:

We understand that the ActualHeight should also be 56.
Thanks in advance.

FYI @agneszitte as I know we had worked on fiddling with these numbers previously

@kazo0 For the ThemeResource property setters, what should we do for the properties that are not specifically used at the moment but available for the control?
cc: @agneszitte

@kazo0 For the ThemeResource property setters, what should we do for the properties that are not specifically used at the moment but available for the control? cc: @agneszitte

Talking about the principal main setters @kazo0 to be clearer (cf. here as an example)

Hi @ADD-Noureddine-Maachi
The fixes are now available with the latest dev build of Uno Themes, but they haven’t yet been released in a stable version of uno.sdk. We’ll need to conduct additional QA to ensure these changes don’t introduce any regressions.
In the meantime, you can test the changes by overriding the Uno Themes version specifically (alongside the current uno.sdk version you're using). To do so, add the following to your Directory.Build.props file:
<PropertyGroup>
    <UnoThemesVersion>5.4.0-dev.18</UnoThemesVersion>
</PropertyGroup>

[!WARNING]
Please remember to remove this override when it’s no longer necessary, once the fixes are included in a stable release of uno.sdk.

Here are the details of the related changes to the Resource Keys with the latest updates:
Material Migration Guide - Upgrading to Uno Themes v-next
Please feel free to reach out to us with any additional feedback as needed.


HstSelect control throws error

Describe the bug
When writing a story and using the HstSelect control, the story stops working. The following error is shown in the console:

In addition, the HstSelect has no intellisense in VSCode.

Reproduction
I've created a new Vue3 project using npm init vue@next, afterwards I've added Histoire to the project using the steps described in the documentation. To get the project to run I had to downgrade vite and @vitejs/plugin-vue as Histoire doesn't yet support Vite 3.
I've copied the BaseButton.vue component from the histoire example and pasted it into the project I just created.
The repository can be found here
System Info
System:
    OS: macOS 12.4
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 693.53 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    npm: 8.15.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
  Browsers:
    Chrome: 103.0.5060.134
    Edge: 103.0.1264.71
    Firefox: 101.0
    Safari: 15.5
    Safari Technology Preview: 16.0
  npmPackages:
    @vitejs/plugin-vue: ^2.3.2 => 2.3.3
    histoire: ^0.8.4 => 0.8.4
    vite: ^2.9.12 => 2.9.12

Used Package Manager
npm
Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
[X] The provided reproduction is a minimal reproducible example of the bug.

I came over with the same problem, it seems that the HstSelect doesn't work even in the online demo. Have you got any idea?

Looks like it works here  and here 🤔

In addition, the HstSelect has no intellisense in VSCode.

Looks like the definition is missing here indeed

Looks like it works here and here 🤔

Yeah, that is what's so odd. On the example website it works like a charm. When I run the Vue3 example on my machine it also works. But when I take the BaseButton component and past it into a fresh Vue3 project with Histoire installed from NPM it fails.

@Akryum I got the same error on a project where the option was reactive, but I tried to create a repro on Stackblitz and couldn't reproduce anymore, it's something related with the floating-vue


[QUESTION] how to use {{TEMPLATE:}} syntax

sorry if this is a stupid question... it says here that {{TEMPLATE:<TEMPLATEPATH>}} allows you to include a template in your format.
i created a template to be used by a capture choice, that looks something like this:
---
title: {{VALUE:Title}}
type: {{VALUE:outline,story,ideas,tasklist}}
somevar: {{TEMPLATE:z/templater/testplate}}
---

however, {{TEMPLATE:z/templater/testplate}} doesn't change to the template contents. am i doing something wrong? should i not be using a capture? apologies once again, i'm really new to this plugin 😅

I also see this behavior using Obsidian 1.0.3 where I use a vanilla Template file for a template choice in this plugin. When I run QuickAdd, the template syntax date:{{DATE}} does not automatically populate as I'd expect when the note is created.

Add the file extension to the end, like so:
somevar: {{TEMPLATE:z/templater/testplate}}


Make POSIX compliant de/activate scripts

Checklist

[ ] Used a personal fork of the feedstock to propose changes
[ ] Bumped the build number (if the version is unchanged)
[ ] Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

Doing my best to duplicate https://github.com/conda-forge/gdal-feedstock/pull/785

@conda-forge-admin, please rerender

@mx-moth, could you take a careful look when you have a chance and make sure this should work?  The activate script will be tested with CI but the deactivate script won't, at least in my past experience.

@conda-forge/udunits2, I believe this is ready to merge but wouldn't mind another pair of eyes on it.

Thanks @mx-moth for having a look and the clarification. I was following the changes made on gdal so I wasn't at all sure what the purpose of the :- was. Good to know that they're unnecessary but harmless.

Thanks for fixing this!
Reading my comment again, I missed a key example in there. ${FOO:-bar} will return the contents of $FOO if $FOO is set and not empty, and will return "bar" if $FOO is unset or is the empty string. The Bash docs have more comprehensive information.


[faudio] Fix hash

[x] Changes comply with the maintainer guide.
[x] SHA512s are updated for each updated download.
[x] The "supports" clause reflects platforms that may be fixed by this new version.
[x] Any fixed CI baseline entries are removed from that file.
[x] Any patches that are no longer applied are deleted from the port's directory.
[x] The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
[x] Only one version is added to each modified port's versions file.

What happened here vs. https://github.com/microsoft/vcpkg/pull/39469/ ? Did upstream change the ref or did we update the SHA incorrectly?

I fixed the REF from a sha to a ${VERSION} on review from @MonicaLiu0311 and for some reason that changed the hash and I forgot to test it again, sorry.

OK, thanks!


Update nomad.rb to 0.1.2

Thank you for the contribution. It was merged directly as 1b2f37871f8655a95d4b70b76a8163cbb0cfdfbe to keep commit history cleaner. Your contribution is still credited to you.

:+1:


Add support to expand/collapse events on Events page

The requirement came to me as: "Improve the page's layout to allow to expand/collapse events where we run multiple things."
I believe that concern is that some events have multiple sessions and it would be nice to collapse these sessions under the event in order not to overwhelm the user/viewer (expand only as necessary, leave collapsed as default).

This seems to already exist as demonstrated by the latest update from @nauerz-ibm in https://github.com/openwhisk/openwhisk.github.io/pull/189
Can we close @mrutkows?

@mrutkows Can you please close this if you think the widget that's currently on the site for the InterConnect sessions suffices?


writing integration tests with busboy

I'm currently trying to write some integration tests using mocha and I'm trying to test streaming in an audio file from busboy into S3. I know as of right now that my fs setup for the test case is correct and is streaming properly when I stream it into my local directory.
As of right now, I think my problem is mocking the headers for the initialization of Busboy. Heres the code for the mocha test case
  it.only('uploads the request to the specified s3 bucket', async () => {
    const mockStream = fs.createReadStream('tests/audioResources/test.mp3');
    const mockUploadAudio = sinon.stub().returns();
    const mockHeaders = {
      host: 'localhost:8000',
      'user-agent': 'curl/7.54.0',
      accept: '*/*',
      'content-length': '444127',
      expect: '100-continue',
      'content-type': 'multipart/form-data; boundary=------------------------dd65445e5a55dcc7',
    };

    const uploadStub = {
      maxPartSize: sinon.stub().returns(),
      concurrentParts: sinon.stub().returns(),
      on: sinon.stub().returns(),
    };

    const services = {
      s3Stream: {
        upload: sinon.stub().returns(uploadStub),
      },
    };
    const config = {
      AudioBucket: 'AudioBucket',
    };
    const audioManager = new AudioManager(services, config);

    await audioManager.upload(mockStream, mockHeaders, mockUploadAudio);

and heres the chunk of code it is testing
  self.upload = async (reqStream, headers, uploadAudio = self.uploadToS3) => {
    const busboy = new Busboy({ headers });
    const busboyPromise = new Promise((resolve, reject) => {
      busboy.on('file', (fieldname, file, filename) => {
        const key = `${Date.now().tostring()}-${filename
          .tolowercase()
          .replace(/\s+/g, '-')
          .replace(/[^\w\-.]+/g, '')}`;

        const params = {
          bucket: AudioBucket,
          key,
        };

        const s3upload = s3Stream.upload(params);
        s3upload.maxpartsize(20971520);
        s3upload.concurrentparts(5);
        s3upload.on('error', (err) => reject(err));
        s3upload.on('uploaded', () => resolve());

        uploadAudio(file, s3upload);
      });
    });

    reqStream.pipe(busboy);

    return busboyPromise;
  };

When I do execute the code above, it just hangs and then times out

It's not enough to just set content-type, you have to actually format the request appropriately. You might take a look at the busboy unit tests as a reference.

I took a look at the unit tests and looked at some docs for request headers and from what I understand, I believe I have the correct media type. As for the boundary, I'm confused as to what should be the correct boundary since they appear to be a string of random characters.

Any help with it?

You need to format your request data appropriately according to the multipart/form-data type. It is not enough to set the appropriate Content-Type header and just send raw binary data.
If you discover you are still having issues after having crafted the body of your request correctly, let me know.


fix typo for

fix typo for

.

Pull Request Test Coverage Report for Build 4401

0 of 0   changed or added relevant lines in 0 files are covered.
17 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.09%) to 69.169%

Files with Coverage Reduction
New Missed Lines
%

cassandra-persistence/src/main/java/com/netflix/conductor/dao/cassandra/CassandraMetadataDAO.java
2
55.44%

postgres-persistence/src/main/java/com/netflix/conductor/dao/postgres/PostgresQueueDAO.java
4
92.11%

postgres-persistence/src/main/java/com/netflix/conductor/dao/postgres/PostgresMetadataDAO.java
11
80.0%

Totals

Change from base Build 4399:
-0.09%

Covered Lines:
12514

Relevant Lines:
18092

💛  - Coveralls


Add getTotaldiscount method to Order

Perhaps we need this. Since we already have getTotalprice.

You can now get total discount of an order with this method:
$order->getTotaldiscount();

To calculate the final price in an order, do this:
$finalprice = $order->getTotalprice() - $order->getTotaldiscount();


Why is there no arg of cutoff for autocontrast at this line?

https://github.com/DeepVoltaire/AutoAugment/blob/17d718251f25c0d9413bf30f91b523907924f33a/autoaugment.py#L218
Hi,
I notice that for autocontrast, the magnitude is not used, and for ImageOps.autocontrast, there is an args named cutoff, which controls the degree of contrast.  If we set cutoff=0, the original image would be returned by this function. Does this mean that the ImageOps.autocontrast is not used at all in both the auto-ml search phase and the verification phase(experiments on imagenet dataset) ?

In the paper they use autocontrast to "Maximize the image contrast, by making the darkest pixel
black and lightest pixel white.". autocontrast() does only return the original image when cutoff=0 when the image has pixel values of 0 and 255. If not, the darkest pixel is mapped to 0, the lightest pixel to white and everything in between is remapped. See the documentation of autocontrast() https://pillow.readthedocs.io/en/stable/reference/ImageOps.html:
"Maximize (normalize) image contrast. This function calculates a histogram of the input image (or mask region), removes cutoff percent of the lightest and darkest pixels from the histogram, and remaps the image so that the darkest pixel becomes black (0), and the lightest becomes white (255)."


Adding permissions for repo directories

Sometimes Odoo needs write access to certain repo directories, such as in the instance of the static directory in Runbot.
One would assume that by creating a script in build.d with a numeric ordering after the repo aggregation in order to set the perms, it would be run after that.
It seems however that the whole custom build directory is run before the one included here, so the permission editing attempt takes place before the repos are aggregated into the directory. Is there a reasoning for this, or am I simply doing something wrong?

Closing for PR #59


UMI !=12bp,can work

If umi length is not 12bp,can sicelore worked?can it found bc and umi ?

The software identifies the length of the UMI from the Illumina data. Definitely also works with 10 nt.  UMIs (the older 10X kits). Should also work with other UMI lengths. However you need to use fixed edit distance since the dynamic edit distances (for preset false assignment percentage) are only supplied for 10 nt. and 12 nt. UMIs in umiMaxEditDistances.xml.
Rainer


[cherry-pick-2.0.1][Bug-7307][MasterServer] timeout event was filtered

this pr close: #7307

Codecov Report

:exclamation: No coverage uploaded for pull request base (2.0.1-release@87e0910). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             2.0.1-release    #7303   +/-   ##
================================================
  Coverage                 ?   31.85%
  Complexity               ?     1565
================================================
  Files                    ?      432
  Lines                    ?    14558
  Branches                 ?     1444
================================================
  Hits                     ?     4638
  Misses                   ?     9470
  Partials                 ?      450

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87e0910...1265569. Read the comment docs.


Migrate to SKSE 2.0.12

Again!

P.S. SKSE runtime is the same though: 1.5.62

P.S. SKSE runtime is the same though: 1.5.62


I get a crash in simulator when trying the example's "Save Static PDF"

am I the only one ?

fixed in https://github.com/derekblair/ILPDFKit/releases/tag/1.1.5


Avoid writing redundant QueueSubmit

The original code wrote one more QueueSubmit for trim QueueSubmits (GFXRECON_CAPTURE_QUEUE_SUBMITS).

CI gfxreconstruct build queued with queue ID 224010.

CI gfxreconstruct build # 4462 running.

CI gfxreconstruct build # 4462 failed.

CI gfxreconstruct build queued with queue ID 224114.

CI gfxreconstruct build # 4464 running.

CI gfxreconstruct build # 4464 passed.


Keep FOC's with respect to risky share for every point on the grid

Codecov Report

Merging #1082 (dc601c8) into master (3c17c27) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1082   +/-   ##
=======================================
  Coverage   73.57%   73.57%
=======================================
  Files          68       68
  Lines       10509    10510    +1
=======================================
+ Hits         7732     7733    +1
  Misses       2777     2777

Impacted Files
Coverage Δ

HARK/ConsumptionSaving/ConsPortfolioModel.py
88.91% <100.00%> (+0.02%)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c17c27...dc601c8. Read the comment docs.

approved by @llorracc via email


when downloading in signed off tab says "download blocked"

says download blocked next to url with red circle
once I clicked on the red circle it asked if I want to allow downloads from this page when I clicked yes it worked (maybe its an edge thing). weird cuz was fine when downloading under new or downloaded.


Autoload

We can now read in an existing TOC when loading a markdown file.

New addition: Activate when markdown grammar gets used

On Activation, check all existing textEditors for markdown and assign a Toc object per textEditor using markdown. (Previously the Toc object was assigned to paneItems).

On addition of new text editors (onDidAddTextEditor callback), check if it's using markdown and assign Toc object if necessary.

Downside: this could be more efficient - first textEditor to load a markdown file runs through init twice (on both activate hook and onDidAddTextEditor callback)

@nok - Just pinging you because this repo hasn't seen any activity in a while, wanted to make sure it didn't get lost

Not ready yet - Selecting Add adds a TOC at the cursor position of every open markdown file.


Support for debuggable web tests

Was finding it impossible to debug web tests when working on IW debugging for web. Was using this for enabling IW debugger, wanted to submit this as a seprate PR.
This was in the launch.json in the past, but for some reason was removed (perhapsh because there was no way to get url working).
The current entry in launch.json doesn't allow debugging, it merely launches the web tests.
Now one can debug web tests end to end.

Codecov Report

Merging #10326 (fcdf7a6) into main (ca8090c) will increase coverage by 0%.
The diff coverage is n/a.

:exclamation: Current head fcdf7a6 differs from pull request most recent head c26a168. Consider uploading reports for the commit c26a168 to get more accurate results

@@          Coverage Diff          @@
##           main   #10326   +/-   ##
=====================================
  Coverage    63%      64%
=====================================
  Files       204      204
  Lines      9295     9295
  Branches   1505     1505
=====================================
+ Hits       5934     5962   +28
+ Misses     2893     2862   -31
- Partials    468      471    +3

Impacted Files
Coverage Δ

src/platform/common/cancellation.ts
62% <0%> (-6%)
:arrow_down:

src/platform/errors/errorHandler.ts
64% <0%> (+1%)
:arrow_up:

.../platform/common/process/pythonEnvironment.node.ts
88% <0%> (+1%)
:arrow_up:

src/platform/api/pythonApi.ts
69% <0%> (+2%)
:arrow_up:

...ommon/process/environmentActivationService.node.ts
69% <0%> (+4%)
:arrow_up:

src/platform/common/process/condaService.node.ts
71% <0%> (+5%)
:arrow_up:


brew-test-bot.rb: tweak missing bottles message when not using jenkins

When not using jenkins (for example when using AWS lambda for linuxbrew,
or even when manually uploading a bottle), the current error message
was "Missing Jenkins variables" when there were no bottles in the dir.
This may happen when no bottle has been built by the CI, or one
is trying to upload bottles from the wrong folder.
The new error messages have been improved and the path is also
displayed, to ease debugging.
There is one assumption here: when JENKINS_HOME is set, one has
to retrieve the bottles first (previously this code path
was triggered by the fact that there were no bottles in the
starting repository)

Thanks @iMichka!


lgamma is not thread safe

Description
We have moved away from using boost functions for most math specialty functions in commit 01bb6645815cd5546701c0575fcb4f38d4a41cca. This affected in particular the log gamma function. Instead we now use the std::lgamma functions which turn out to cause problems in threading contexts. The behaviour is generally undefined wrt to threading safety, see here.
On macOS Mojave the lgamma implementation of clang will introduce locking of a global mutex as can be seen by terrible scaling of multi-threaded benchmarks, see here.
This is a large problem for any threading based applications involving Stan (parallel chains or httpstan).
Example
See the discourse thread linked above for an example.
Expected Output
We should use a thread-safe version of the lgamma - and ideally one which does not lock a global mutex which drastically limits parallelisation performance under threading. At the worst we should distinguish between threading and non-threading.
Right now the behaviour of lgamma when using threads is implementation specific and generally undefined to my understanding.
Current Version:
v2.19.1

Are you suggesting using the Boost version of lgamma?  It's an easy fix, but we have to be careful again about edge conditions and where we want to throw.  I still don't know that exception behavior for our math library has been determined, so I'm punting to @seantalts and @syclik for clarification.

It's whatever the doc string for lgamma says: http://mc-stan.org/math/d4/d84/namespacestan_1_1math.html#aead76f03bdbc60484ad760fc31bad40f

I'm confused.  The doc string for lgamma doesn't say whether to use Boost or not to implement it.

Or do you mean which errors it throws, which it is also silent about.
Are you saying that our API is defined by these incomplete doc strings?

On May 23, 2019, at 2:37 PM, Bob Carpenter<EMAIL_ADDRESS>wrote:
I'm confused.  The doc string for lgamma doesn't say whether to use Boost or not to implement it.

Yeah, what we promise in the doxygen comments is what we're holding ourselves to in theory.
Since it doesn't mention exceptions it seems like we have made no promises there. Would it be hard to match whatever behavior we currently have?

Yes, I am suggesting to revert to commit hash I am quoting above where we replace boost with the std version. However, another area for potential trouble is performance... from my quick tests the boost version is 3x slower than the std version. While the boost version has a well defined precision according to a policy, the std version does not let us control this in any way. Thus, we may have to balance performance vs accuracy, but we don't know where we stand right now.
I can start on a PR where I revert the above commit.

We should make a note of how the behavior changes just to be polite and add it to the release notes one day (maybe put a section on that in the top of the PR or issue, something like "for release notes: lgamma now throws exceptions..." or whatever. I think that seems reasonable. Another weird thing we could do would be to use std::lgamma if we're not in threading mode, but I think that would be too confusing.

I was hoping instead of laying down a letter of the law based on our spotty code documentation, that we'd be moving to consistent treatment of edge cases (+inf, -inf, poles, NaN, and out-of-domain inputs).  I also hoped we'd be adding documentation to clarify behavior rather than avoiding it to allow us wiggle room w.r.t. backward compatibility.

That sounds good, why don't you write up a proposal for how you want to do that as a design doc?

Just linking previous discussion

@wds15: does std::lgamma misbehave with threading? Can you write a test that exhibits the failure? For a bigger question: what other parts of the library aren't really going to work with threading?
I think the issue itself could use more work. What is the ideal outcome? Will using std::lgamma with our current compilers fail us? (Or is it just slow?) This should only affect threaded applications where a Stan program is written with lgamma in it?

That sounds good, why don't you write up a proposal for how you want to do that as a design doc?

@seantalts: I'm not sure how a policy discussion would fall into a design doc (for code)? Maybe my imagination isn't as good as yours.

@seantalts: I'm not sure how a policy discussion would fall into a design doc (for code)? Maybe my imagination isn't as good as yours.

Yeah, design doc (for code) is not the right kind - but some kind of Request For Comments draft would probably be a good way to get traction on that issue.

@syclik std::lgamma is not guaranteed to be thread-safe as per documentation. In fact you get some implementation on a given platform which will behave in a non-defined behaviour wrt to threading. On my macOS system the implementation obviously uses internally a global mutex to access a global variable. I am not sure how I can write a test for such an internal mechanism of the function (maybe by using negative and positive arguments, but I do not know that). Writing a test for this is very hard - right now I could only detect it using performance benchmarks which do not scale with multiple threads, but that isn't a good unit test, right?
Using std::lgamma has the disadvantages of

having undefined behaviour across platforms wrt to threading. On macOS performance is seriously hampered when used in parallel due to an internal mutex.
the output accuracy is uncontrolled - at least I cannot find any notes on the accuracy of that function

However, when I drop-in replaced std::lgamma with the boost version, then the boost version was ~3x slower than std.
The best outcome is to use the boost version of lgamma for all our applications and hopefully when the boost version is asked to output similar accuracy as the std version then the speed is the same of both functions.
And yes, this problem is with probably a few more gamma functions and maybe even with more functions which we do not know about yet.

It's not clear what we're talking about for thread safety here.  You mean the ability to concurrently call a function?
The container classes (std::vector, Eigen::Matrix) are not thread safe, for example, but I don't think our library ever accesses a container concurrently.  The log density accumulator isn't thread safe.
None of our autodiff classes are thread safe.  Our autodiff functionals are not thread safe.
Static constant initialization is guaranteed to be thread safe.  But nothing with a mutable static is going to be thread safe.
Is the Boost version of lgamma thread safe?  Does it allow accuracy control?  I don't think Stan has any requirements for accuracy.
Here's what the spec says for lgamma:

The POSIX version of lgamma is not thread-safe: each execution of the function stores the sign of the gamma function of arg in the static external variable signgam. Some implementations provide lgamma_r, which takes a pointer to user-provided storage for singgam as the second parameter, and is thread-safe.

Neither tgamma nor digamma have similar qualifications about thread safety.

On May 27, 2019, at 4:00 AM, wds15<EMAIL_ADDRESS>wrote:
@syclik std::lgamma is not guaranteed to be thread-safe as per documentation. In fact you get some implementation on a given platform which will behave in a non-defined behaviour wrt to threading. On my macOS system the implementation obviously uses internally a global mutex to access a global variable. I am not sure how I can write a test for such an internal mechanism of the function (maybe by using negative and positive arguments, but I do not know that). Writing a test for this is very hard - right now I could only detect it using performance benchmarks which do not scale with multiple threads, but that isn't a good unit test, right?
Using std::lgamma has the disadvantages of
• having undefined behaviour across platforms wrt to threading. On macOS performance is seriously hampered when used in parallel due to an internal mutex.
• the output accuracy is uncontrolled - at least I cannot find any notes on the accuracy of that function
However, when I drop-in replaced std::lgamma with the boost version, then the boost version was ~3x slower than std.
The best outcome is to use the boost version of lgamma for all our applications and hopefully when the boost version is asked to output similar accuracy as the std version then the speed is the same of both functions.
And yes, this problem is with probably a few more gamma functions and maybe even with more functions which we do not know about yet.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

That's right - all of stan-math is not thread safe unless you run it with STAN_THREADS turned on. In that case we can run things concurrently with map_rect. In this setting we have a problem with std::lgamma for these reasons:

There is no thread safety guarantee. So concurrent calls to std::lgamma from concurrent map_rect context's will be problematic.
It turns out that on macOS the behavior of std::lgamma whenever threads are used is such that a global mutex is used to synchronize the calls (likely to deal with the sign of the gamma function). This synchronization makes the implementation on macOS per se thread safe, but it kills off any performance gains through parallelization.
The worst thing is that the thread behavior of that function is implementation dependent (OS, compiler, libstdc++, whatever)

Does that explain?

I still have the question of whether Boost's lgamma is thread safe.  Is the proposal just to use that instead of std::lgamma?
STAN_THREADS only makes the memory access thread safe.  It doesn't make things like containers (Eigen::Matrix or std::vector) or our stan::math::vari instances thread safe for mutability.

On May 28, 2019, at 1:37 PM, wds15<EMAIL_ADDRESS>wrote:
That's right - all of stan-math is not thread safe unless you run it with STAN_THREADS turned on. In that case we can run things concurrently with map_rect. In this setting we have a problem with std::lgamma for these reasons:
• There is no thread safety guarantee. So concurrent calls to std::lgamma from concurrent map_rect context's will be problematic.
• It turns out that on macOS the behavior of std::lgamma whenever threads are used is such that a global mutex is used to synchronize the calls (likely to deal with the sign of the gamma function). This synchronization makes the implementation on macOS per se thread safe, but it kills off any performance gains through parallelization.
• The worst thing is that the thread behavior of that function is implementation dependent (OS, compiler, libstdc++, whatever)
Does that explain?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

STAN_THREADS only makes the memory access thread safe.  It doesn't make things like containers (Eigen::Matrix or std::vector) or our stan::math::vari instances thread safe for mutability.

That's why we only support threading via map_rect :D

On May 28, 2019, at 2:00 PM, seantalts<EMAIL_ADDRESS>wrote:
STAN_THREADS only makes the memory access thread safe. It doesn't make things like containers (Eigen::Matrix or std::vector) or our stan::math::vari instances thread safe for mutability.
That's why we only support threading via map_rect :D

My understanding is that std::lgamma's lack of thread safety becomes a problem precisely because map_rect might call it from multiple threads.  So I believe map_rect imposes thread-safety requirements on every single Stan function that might be evaluated in a Stan program.  Generally, we haven't allowed side effects other than the autodiff stack, so that's not a problem.  But this is partly why proposals to do things like statically store state to restart solvers is going to be tricky.  And why we better make sure every static local variable is const.
As things stand, the output of print(...) will get garbled as the underlying I/O is thread safe but not atomic at the message level.
Aren't other issues/PRs in the works to relax the restriction of threading to map_rect?  Such as allowing parallel for loops?

My understanding is that std::lgamma's lack of thread safety becomes a problem precisely because map_rect might call it from multiple threads.

Yep. Just highlighting that the only sense of "thread safety" we need to worry about is via map_rect. If the design doc for parallel for goes through, that will need to use the same mechanisms, which I believe is the plan.

Things need to be safe if concurrently executed in different local contexts....and static variables can be a source of problems if these are not const, yes.

@bob-carpenter is right. map_rect is imposing thread-safety requirements on any Stan function that might be evaluated in the function passed to map_rect. We haven't been so careful about this since thread safety wasn't a requirement, but I think we should be much more careful going forward.
Fortunately, I think lgamma is one of the few functions we would use that are known to not be thread-safe. See this list http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_09_01. (I see it linked everywhere... I don't know if it covers all of the standard library or if we really need to be digging through the doc there.)
We don't have this problem with MPI because it uses separate processes.
Part of me thinks we should reevaluate including threading for now while we sort out these problems; we essentially need to search through all library functions we use to see if they're not thread-safe. But we can pick that discussion up on discourse.

For now, I think it's reasonable to describe the qualities of lgamma that we want, to create some tests for it, and to verify that the boost version has the characteristics that we want. Then switch to that implementation.

I am not sure how I can write a test for such an internal mechanism of the function (maybe by using negative and positive arguments, but I do not know that). Writing a test for this is very hard - right now I could only detect it using performance benchmarks which do not scale with multiple threads, but that isn't a good unit test, right?

@wds15: we need to write a test that flexes what we think could be an issue when running the function with threading. Yes, it might not be easy, but this is a necessary condition. How else will you prevent someone in the future from looking at boost::lgamma and thinking... "oh, I'll switch that to std::lgamma"? And maybe we can replace it in the future. Or we might be able to avoid a problem with a future implementation.
Re: nomenclature. I don't care if it's not called a unit test, but it's what we need to test.

The link says

The POSIX version of lgamma is not thread-safe: each execution of the function stores the sign of the gamma function of arg in the static external variable signgam.
which I have always thought was a non-issue for us because nothing in Stan Math is or should be writing or reading signgam.

@bgoodri: that makes sense. Does that mean it's the results will be safe with multiple threads? Does it explain why @wds15 has seen a performance hit?

The title of this issue shouldn't be "lgamma is not thread safe"; rather it should be "signgam is not thread safe". So, if it is true that no one is using signgam then the results are safe and I wouldn't be surprised if the std version were faster than the Boost version. The C++ standards gurus also require accuracy within 1 ULP or something, so we are probably fine on that front.
For statistics in general and HMC in particular, I have a hard time thinking of an example where someone would want to access signgam. Typically, we already are requiring the input to be non-negative; otherwise HMC would have the problems it already has with abs or square on input that can be either negative or positive. Worse, the gamma function is crazy for negative input with oscillations and singularities, so I don't see how anyone would use Stan to do statistical inference unless the input was already constrained to be non-negative.
We could introduce a lgamma_r function to Stan Math and / or have stan::math::lgamma set signgam to -2147483647.

@bob-carpenter  re: boost::math and thread safety: https://www.boost.org/doc/libs/1_70_0/libs/math/doc/html/math_toolkit/threads.html

Tests are great if feasible.  It can be really hard to test these threading bugs---I've seen cases that take 48 hours on 72 concurrent threads to arise.

How else will you prevent someone in the future from looking at boost::lgamma and thinking... "oh, I'll switch that to std::lgamma"?

lgamma isn't special---we have the same requirements on all of our functions.  The only difference is that it has a tempting replacement in std::lgamma.  My suggestion is to doc where Boost's version is used saying that the std:: version isn't thread safe.
I don't know how to stop people from calling std::lgamma elsewhere when they shouldn't.  Currently, the only place std::lgamma is used is in stan/math/prim/scal/fun/lgamma.hpp and that should probably be the only place Boost's gamma is used.

https://www.boost.org/doc/libs/1_70_0/libs/math/doc/html/math_toolkit/threads.html

That's convenient.  It just says:

The library is fully thread safe and re-entrant for all functions regards of the data type they are instantiated on. Thread safety limitations relating to user defined types present in previous releases (prior to 1.50.0) have been removed.

lgamma isn't special---we have the same requirements on all of our functions. The only difference is that it has a tempting replacement in std::lgamma.

What other standard functions have the side effect of setting a global variable like signgam?

How else will you prevent someone in the future from looking at boost::lgamma and thinking... "oh, I'll switch that to std::lgamma"?

By imposing thread safety requirements and assuming people know the language.  It's how we do everything.  We can't guard against generic programmer error.  We have exactly the same problem with every one of our functions, though they don't come with such a tempting replacement.
If you can formulate a test that's reliable, that's great, but it's a notoriously hard problem in a multithreaded context to trigger bugs.  I've seen threading bugs that took on the order of days to trigger given dozens of concurrent threads.

None that I can find searching for thread safety.  None of the other gamma functions have the same qualification about threading in their doc.  I have no idea why lgamma is different than the other lib functions.

On May 30, 2019, at 2:17 PM, bgoodri<EMAIL_ADDRESS>wrote:
lgamma isn't special---we have the same requirements on all of our functions. The only difference is that it has a tempting replacement in std::lgamma.
What other standard functions have the side effect of setting a global variable like signgam?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

What other standard functions have the side effect of setting a global variable like signgam?

@bgoodri: it looks like functions that are related (I'm seeing the same list on different sites and can find a source if you want it):
gamma(), lgamma(),lgammaf(),lgammal()
I don't know if there are other functions in Eigen or elsewhere that have these behaviors.
More importantly, this means that any Math function implementation can't really use static variables to compute things. This is a new requirement. That would have been bad practice, but we haven't explicitly checked for it or forbidden it. The only places where I could think we might even attempt to do that is in optimized code. There's a lot of cleverness there with separate memory management and we should probably comb through that very carefully.

By imposing thread safety requirements and assuming people know the language.  It's how we do everything.  We can't guard against generic programmer error.  We have exactly the same problem with every one of our functions, though they don't come with such a tempting replacement.

Yup. What's the best way to impose thread safety requirements?

If you can formulate a test that's reliable, that's great, but it's a notoriously hard problem in a multithreaded context to trigger bugs.  I've seen threading bugs that took on the order of days to trigger given dozens of concurrent threads.

@wds15 was able to demonstrate that std::lgamma performs differently from boost::lgamma with a few threads. That's what generated this issue. That should be one of the tests, if not the test.

@bgoodri: it looks like functions that are related (I'm seeing the same list on different sites and can find a source if you want it):
gamma(), lgamma(),lgammaf(),lgammal()

gamma() isn't in the standard library in C++.
lgammaf() and lgammal() are just the float and long form argument forms of lgamma().

By imposing thread safety requirements and assuming people know the language. It's how we do everything. We can't guard against generic programmer error. We have exactly the same problem with every one of our functions, though they don't come with such a tempting replacement.
Yup. What's the best way to impose thread safety requirements?

My response was above and I'll stick by it.  We can't test this
programmatically in a proactive way.

If you can formulate a test that's reliable, that's great, but it's a notoriously hard problem in a multithreaded context to trigger bugs. I've seen threading bugs that took on the order of days to trigger given dozens of concurrent threads.
@wds15 was able to demonstrate that std::lgamma performs differently from boost::lgamma with a few threads. That's what generated this issue. That should be one of the tests, if not the test.

Great---that's what I meant by being able to formulate a reliable test.

A test for this will make everything brittle...I need to launch 4 threads and observe that the speed needs to be 4x as fast. Doubling was not enough...this implies I need 4 idle cores for the test to run fine...on top of that this the behavior on mac...I don’t know how things are under Linux or windows...it could be different there.


⚠ Bofrost.at has degraded performance

In b2cae8f, Bofrost.at (https://www.bofrost.at) experienced degraded performance:

HTTP code: 200
Response time: 1186 ms

Resolved: Bofrost.at performance has improved in 7121b14.


Prominently display the emulator URL at startup

Prominently display the emulator URL at startup. Currently the ports used by auth server and others are also printed, so we need to make the actual emulator port stand out.

I suggest using the chalk package for that.

Related #111

Closed in #111