text:
test/codegen: brittle on multi-line go expressions that get coalesced into single instructions

The following test passed with the very long line,
func load_byte8_uint64_idx(s []byte, idx int) uint64 {
// arm64:`MOVD\s\(R[0-9]+\)\(R[0-9]+<<3\)`,-`ORR\tR[0-9]+<<[0-9]+`
	return uint64(s[idx<<3]) | uint64(s[(idx<<3)+1])<<8 | uint64(s[(idx<<3)+2])<<16 | uint64(s[(idx<<3)+3])<<24 | uint64(s[(idx<<3)+4])<<32 | uint64(s[(idx<<3)+5])<<40 | uint64(s[(idx<<3)+6])<<48 | uint64(s[(idx<<3)+7])<<56
}

But if the long line was broken two lines, the test would fail, though expected asm had been generated.
func load_byte8_uint64_idx(s []byte, idx int) uint64 {
// arm64:`MOVD\s\(R[0-9]+\)\(R[0-9]+<<3\)`,-`ORR\tR[0-9]+<<[0-9]+`
	return uint64(s[idx<<3]) | uint64(s[(idx<<3)+1])<<8 | uint64(s[(idx<<3)+2])<<16 | uint64(s[(idx<<3)+3])<<24 |
		uint64(s[(idx<<3)+4])<<32 | uint64(s[(idx<<3)+5])<<40 | uint64(s[(idx<<3)+6])<<48 | uint64(s[(idx<<3)+7])<<56
}

with error message
2018/04/25 13:02:09 "".load_byte8_uint64_idx STEXT size=176 args=0x28 locals=0x8
0x0000 00000 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	TEXT	"".load_byte8_uint64_idx(SB), $8-40
0x0000 00000 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD	16(g), R1
0x0004 00004 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD	RSP, R2
0x0008 00008 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	CMP	R1, R2
0x000c 00012 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	BLS	160
0x0010 00016 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD.W	R30, -16(RSP)
0x0014 00020 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	FUNCDATA	ZR, gclocals·6b4b7e46e7c3e785dae149c064ae0142(SB)
0x0014 00020 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	FUNCDATA	$1, gclocals·69c1753bd5f81501d95132d08af04464(SB)
0x0014 00020 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	MOVD	"".s+8(FP), R0
0x0018 00024 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	MOVD	"".idx+24(FP), R1
0x001c 00028 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CMP	R1<<3, R0
0x0020 00032 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	BLS	152
0x0024 00036 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	LSL	$3, R1, R2
0x0028 00040 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	ADD	$1, R2, R3
0x002c 00044 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CMP	R0, R3
0x0030 00048 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	BHS	152
0x0034 00052 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	ADD	$2, R2, R3
0x0038 00056 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CMP	R0, R3
0x003c 00060 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	BHS	152
0x0040 00064 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	ADD	$3, R2, R3
0x0044 00068 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CMP	R0, R3
0x0048 00072 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	BHS	152
0x004c 00076 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	ADD	$4, R2, R3
0x0050 00080 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	CMP	R0, R3
0x0054 00084 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	BHS	144
0x0058 00088 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	ADD	$5, R2, R3
0x005c 00092 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	CMP	R0, R3
0x0060 00096 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	BHS	144
0x0064 00100 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	ADD	$6, R2, R3
0x0068 00104 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	CMP	R0, R3
0x006c 00108 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	BHS	144
0x0070 00112 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	ADD	$7, R2, R2
0x0074 00116 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	CMP	R0, R2
0x0078 00120 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	BHS	144
0x007c 00124 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	MOVD	"".s(FP), R0
0x0080 00128 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	MOVD	(R0)(R1<<3), R0
0x0084 00132 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	MOVD	R0, "".~r2+32(FP)
0x0088 00136 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	MOVD.P	16(RSP), R30
0x008c 00140 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	RET	(R30)
0x0090 00144 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	PCDATA	ZR, $1
0x0090 00144 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	CALL	runtime.panicindex(SB)
0x0094 00148 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	UNDEF
0x0098 00152 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	PCDATA	ZR, $1
0x0098 00152 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CALL	runtime.panicindex(SB)
0x009c 00156 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	UNDEF
0x00a0 00160 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	NOP
0x00a0 00160 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	PCDATA	$0, $-1
0x00a0 00160 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD	R30, R3
0x00a4 00164 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	CALL	runtime.morestack_noctxt(SB)
0x00a8 00168 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	JMP	0
0x0000 81 0b 40 f9 e2 03 00 91 5f 00 01 eb a9 04 00 54  ..@....._......T
0x0010 fe 0f 1f f8 e0 13 40 f9 e1 1b 40 f9 1f 0c 01 eb  ......@...@.....
0x0020 c9 03 00 54 22 f0 7d d3 43 04 00 91 7f 00 00 eb  ...T".}.C.......
0x0030 42 03 00 54 43 08 00 91 7f 00 00 eb e2 02 00 54  B..TC..........T
0x0040 43 0c 00 91 7f 00 00 eb 82 02 00 54 43 10 00 91  C..........TC...
0x0050 7f 00 00 eb e2 01 00 54 43 14 00 91 7f 00 00 eb  .......TC.......
0x0060 82 01 00 54 43 18 00 91 7f 00 00 eb 22 01 00 54  ...TC......."..T
0x0070 42 1c 00 91 5f 00 00 eb c2 00 00 54 e0 0f 40 f9  B..._......T..@.
0x0080 00 78 61 f8 e0 1f 00 f9 fe 07 41 f8 c0 03 5f d6  .xa.......A..._.
0x0090 00 00 00 94 00 17 a7 be 00 00 00 94 00 17 a7 be  ................
0x00a0 e3 03 1e aa 00 00 00 94 d6 ff ff 17 00 00 00 00  ................
rel 140+0 t=11 +0
rel 144+4 t=10 runtime.panicindex+0
rel 152+4 t=10 runtime.panicindex+0
rel 164+4 t=10 runtime.morestack_noctxt+0
# go run run.go -- codegen/memcombine.go
codegen/memcombine.go:125: linux/arm64/: opcode not found: "^MOVD\\s\\(R[0-9]+\\)\\(R[0-9]+<<3\\)"

If you split the OR sequence into two lines, the MOVD   (R0)(R1<<3), R0 instruction you are matching gets a line number from the last line of Go code, so the regexp should go near the last line. This is what happens in this other codegen test:
1 func zero_byte_16(b []byte) {
2 	_ = b[15]
3	b[0], b[1], b[2], b[3] = 0, 0, 0, 0
4	b[4], b[5], b[6], b[7] = 0, 0, 0, 0
5	b[8], b[9], b[10], b[11] = 0, 0, 0, 0
6	b[12], b[13], b[14], b[15] = 0, 0, 0, 0 // arm64:"STP",-"MOVB",-"MOVH",-"MOVW"
7  }

We want all the zeroing to be coalesced into an STP on arm64, but when that happens the compiler assigns line number 6 (where the last assignment is) to the STP instruction, and for this reason the regexp needs to go near the last line.
It's a limitation of the current approach (if for some reason the compiler starts to assign line number 3 to the STP, we'll need to update the test); but I'm not sure how you would protect from this.

I see the reason. Thank you.

Sorry, still failed.
func load_byte8_uint64_idx(s []byte, idx int) uint64 {
	return uint64(s[idx<<3]) | uint64(s[(idx<<3)+1])<<8 | uint64(s[(idx<<3)+2])<<16 | uint64(s[(idx<<3)+3])<<24 |
		uint64(s[(idx<<3)+4])<<32 | uint64(s[(idx<<3)+5])<<40 | uint64(s[(idx<<3)+6])<<48 |
		uint64(s[(idx<<3)+7])<<56 // arm64:`MOVD\t\(R[0-9]+\)\(R[0-9]+<<3\)`,-`ORR\tR[0-9]+<<[0-9]+`
	}

0x0000 00000 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	TEXT	"".load_byte8_uint64_idx(SB), $8-40
0x0000 00000 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD	16(g), R1
0x0004 00004 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD	RSP, R2
0x0008 00008 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	CMP	R1, R2
0x000c 00012 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	BLS	168
0x0010 00016 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD.W	R30, -16(RSP)
0x0014 00020 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	FUNCDATA	ZR, gclocals·6b4b7e46e7c3e785dae149c064ae0142(SB)
0x0014 00020 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	FUNCDATA	$1, gclocals·69c1753bd5f81501d95132d08af04464(SB)
0x0014 00020 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	MOVD	"".s+8(FP), R0
0x0018 00024 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	MOVD	"".idx+24(FP), R1
0x001c 00028 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	CMP	R1<<3, R0
0x0020 00032 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	BLS	160
0x0024 00036 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	LSL	$3, R1, R2
0x0028 00040 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	ADD	$1, R2, R3
0x002c 00044 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	CMP	R0, R3
0x0030 00048 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	BHS	160
0x0034 00052 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	ADD	$2, R2, R3
0x0038 00056 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	CMP	R0, R3
0x003c 00060 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	BHS	160
0x0040 00064 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	ADD	$3, R2, R3
0x0044 00068 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	CMP	R0, R3
0x0048 00072 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	BHS	160
0x004c 00076 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	ADD	$4, R2, R3
0x0050 00080 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CMP	R0, R3
0x0054 00084 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	BHS	152
0x0058 00088 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	ADD	$5, R2, R3
0x005c 00092 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CMP	R0, R3
0x0060 00096 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	BHS	152
0x0064 00100 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	ADD	$6, R2, R3
0x0068 00104 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CMP	R0, R3
0x006c 00108 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	BHS	152
0x0070 00112 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	ADD	$7, R2, R2
0x0074 00116 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	CMP	R0, R2
0x0078 00120 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	BHS	144
0x007c 00124 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	MOVD	"".s(FP), R0
0x0080 00128 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	MOVD	(R0)(R1<<3), R0
0x0084 00132 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	MOVD	R0, "".~r2+32(FP)
0x0088 00136 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	MOVD.P	16(RSP), R30
0x008c 00140 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	RET	(R30)
0x0090 00144 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	PCDATA	ZR, $1
0x0090 00144 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	CALL	runtime.panicindex(SB)
0x0094 00148 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:126)	UNDEF
0x0098 00152 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	PCDATA	ZR, $1
0x0098 00152 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	CALL	runtime.panicindex(SB)
0x009c 00156 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:125)	UNDEF
0x00a0 00160 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	PCDATA	ZR, $1
0x00a0 00160 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	CALL	runtime.panicindex(SB)
0x00a4 00164 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	UNDEF
0x00a8 00168 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:124)	NOP
0x00a8 00168 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	PCDATA	$0, $-1
0x00a8 00168 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	MOVD	R30, R3
0x00ac 00172 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	CALL	runtime.morestack_noctxt(SB)
0x00b0 00176 (/home/local/SPREADTRUM/ben.shi/Desktop/go/test/codegen/memcombine.go:123)	JMP	0
0x0000 81 0b 40 f9 e2 03 00 91 5f 00 01 eb e9 04 00 54  ..@....._......T
0x0010 fe 0f 1f f8 e0 13 40 f9 e1 1b 40 f9 1f 0c 01 eb  ......@...@.....
0x0020 09 04 00 54 22 f0 7d d3 43 04 00 91 7f 00 00 eb  ...T".}.C.......
0x0030 82 03 00 54 43 08 00 91 7f 00 00 eb 22 03 00 54  ...TC......."..T
0x0040 43 0c 00 91 7f 00 00 eb c2 02 00 54 43 10 00 91  C..........TC...
0x0050 7f 00 00 eb 22 02 00 54 43 14 00 91 7f 00 00 eb  ...."..TC.......
0x0060 c2 01 00 54 43 18 00 91 7f 00 00 eb 62 01 00 54  ...TC.......b..T
0x0070 42 1c 00 91 5f 00 00 eb c2 00 00 54 e0 0f 40 f9  B..._......T..@.
0x0080 00 78 61 f8 e0 1f 00 f9 fe 07 41 f8 c0 03 5f d6  .xa.......A..._.
0x0090 00 00 00 94 00 17 a7 be 00 00 00 94 00 17 a7 be  ................
0x00a0 00 00 00 94 00 17 a7 be e3 03 1e aa 00 00 00 94  ................
0x00b0 d4 ff ff 17 00 00 00 00 00 00 00 00 00 00 00 00  ................
rel 140+0 t=11 +0
rel 144+4 t=10 runtime.panicindex+0
rel 152+4 t=10 runtime.panicindex+0
rel 160+4 t=10 runtime.panicindex+0
rel 172+4 t=10 runtime.morestack_noctxt+0
# go run run.go -- codegen/memcombine.go

codegen/memcombine.go:126: linux/arm64/: opcode not found: "^MOVD\\t\\(R[0-9]+\\)\\(R[0-9]+<<3\\)"

Though there is a walkaround, I still expect to see the removal of such a limitation in the future.

Can you include the line numbers in the Go code like I did above:
1 func zero_byte_16(b []byte) {
2 	_ = b[15]
3	b[0], b[1], b[2], b[3] = 0, 0, 0, 0
4	b[4], b[5], b[6], b[7] = 0, 0, 0, 0
5	b[8], b[9], b[10], b[11] = 0, 0, 0, 0
6	b[12], b[13], b[14], b[15] = 0, 0, 0, 0 // arm64:"STP",-"MOVB",-"MOVH",-"MOVW"
7  }

?

123 func load_byte8_uint64_idx(s []byte, idx int) uint64 {
124 	return uint64(s[idx<<3]) | uint64(s[(idx<<3)+1])<<8 | uint64(s[(idx<<3)+2])<<16 | uint64(s[(idx<<3)+3])<<24 |
125 		uint64(s[(idx<<3)+4])<<32 | uint64(s[(idx<<3)+5])<<40 | uint64(s[(idx<<3)+6])<<48 |
126 		uint64(s[(idx<<3)+7])<<56 // arm64:`MOVD\t\(R[0-9]+\)\(R[0-9]+<<3\)`,-`ORR\tR[0-9]+<<[0-9]+`
127 	}

Oh, so MOVD	(R0)(R1<<3), R0 gets assigned to the middle line (125)? Is this expected?
Anyway I suggest keeping the whole expression on one line and the regexp on the line above, for now.

(I've changed the title to describe the general issue we're having here).

It passed with a quite strange form
123 func load_byte8_uint64_idx(s []byte, idx int) uint64 {
124 	return uint64(s[idx<<3]) | uint64(s[(idx<<3)+1])<<8 | uint64(s[(idx<<3)+2])<<16 | uint64(s[(idx<<3)+3])<<24 |
125 	uint64(s[(idx<<3)+4])<<32 | uint64(s[(idx<<3)+5])<<40 | uint64(s[(idx<<3)+6])<<48 | // arm64:`MOVD\s\(R[0-9]+\)\(R[0-9]+<<3\)`,-`ORR\tR[0-9]+<<[0-9]+`
126 	uint64(s[(idx<<3)+7])<<56
127 }

I think that the best solution in this case is simply to keep everything in one line. It's a little harder to read but it will be stable, with no surprises in the future.
Let's see how often it happens before designing a workaround.

CC @randall77

I agree with @rasky , it's slightly painful to make it one long line, but probably simpler than complicating the matching algorithm.
I'll close for now, but feel free to reopen if it starts getting more painful, or you see a simple way of modifying the matching algorithm.


text:
cmd/go: add 'require test' section to go.mod

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version devel +479da24aac 2018-08-10 00:47:31 +0000 linux/amd64
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (go env)?
Linux/amd64
What did you do?
cd $GOPATH/github.com/OneOfOne/xxhash
gotip mod tidy

What did you expect to see?
● cat go.mod
module github.com/OneOfOne/xxhash

What did you see instead?
module github.com/OneOfOne/xxhash

require (
	github.com/cespare/xxhash v1.0.0
	github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72 // indirect
)

Those 2 deps are only used in *_test.go files.
This isn't a bug really but more of a feature request, is there any way to split those sections into require (...) and require test (...)?
Without that split, go get -t is pretty much meaningless.
If there's interest, I'd like to take a stab at it.

@googlebot please add modules

@gopherbot modules

Yes please. This mirrors maven's notion of scopes; the set of dependencies for a build scope are different to the set of dependencies for a test scope.

CC @rsc @bcmills

In a sense, go get -t is now the default.
The module download is split into two parts: downloading the go.mod and downloading the actual code. If you have dependencies only needed for tests, then they will show up in your go.mod, and go get will download their go.mods, but it will not download their code. The test-only dependencies get downloaded only when you need it, such as the first time you run go test.
This applies not just to test-only dependencies but also os-specific dependencies. For example if you use logrus but don't use Solaris, then the golang.org/x/sys go.mod needs to be downloaded, but not the code in that repo.
When you do split things out explicitly, then you'd have different version selection results for the different "scopes". You could potentially be using one version for a build and then get a different version for a test. That would be quite unfortunate. Having one unified go.mod avoids that potential problem.
So you're getting the fine-grained separation you want already, for free*, without having to maintain any explicit scopes, and without any possibility of inconsistency.
* The asterisk is that right now because we're still fetching git repos, even to get the go.mod, what I said isn't true. But once we have a better story for a proxy, it will become true, all with no effort. Because we do have a plan to get there, though, we're not planning to add any of these kinds of scopes as a temporary workaround. They'd just cause needless pain in the long run.

@rsc , wouldn't it be helpful to mark in any way dependencies that are only used for tests? I find it quite confusing to have dependencies in the go.mod that are not actually present in run time.

I find it quite confusing to have dependencies in the go.mod that are not actually present in run time.

They are present at run time, if you run go test all, and go mod why should help clear up the confusion for any given dependency.

I think it would be more clear if there were a way to know from the go tool or the go.mod which requirements are only for testing,  and that doing this would not violate the problem of inconsistent scopes.    It is in any event helpful to be clear about really why a requirement is there, as requirements without test fall under a different set of considerations for a module maintainer than requirements for tests.  (The consumer of the former is more the end user and the later more developers).
Note, this is not to say that I think splitting the requirements is good, as @rsc noted that causes problems.  But explaining why requirements are there without mentioning scopes like testing is not ideal.
Also there is a question on go-nuts where there is some concern about vendoring overhead, but the justification  from @rsc
that they are or will be free
"""

The asterisk is that right now because we're still fetching git repos, even to get the go.mod, what I said isn't true. But once we have a better story for a proxy, it will become true, all with no effort. Because we do have a plan to get there, though, we're not planning to add any of these kinds of scopes as a temporary workaround. They'd just cause needless pain in the long run.
"""
does not address the relationship to vendoring.  Sorry I don't have an answer for whether there is vendoring overhead.

redirecting my comment above to related #26955

Having visibility into this is really important. Some of my test dependencies transitive packages are for things that I think I really really don't want in my runtime, and I would prefer my downstreams not see the pile of cruft that my use of GoConvey brings in, or at least see it with the explicit understanding that I'm not using this in the standard build.
For now this is actually driving me away from using certain 3rd party testing packages, because I consider their rather large dependency graph as pollution in my go.mod file.

@gdamore, this issue is closed, which means that nobody will be revisiting it when it is time to plan work. Please continue discussion on one of the related issues that are still open (such as #26955).

source: gharchive/issue

authors: OneOfOne bcmills davecheney gdamore ianlancetaylor posener rsc wsc1

repo: golang/go

license_type: permissive

license_source: github-api


text:
cmd/compile: impossible slice access is compiled in spite of always panicking

What version of Go are you using (go version)?
go version go1.11 windows/amd64
What operating system and processor architecture are you using (go env)?
set GOARCH=amd64
set GOBIN=
set GOCACHE=...
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=...
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=... -gno-record-gcc-switches
What did you do?
I compiled the following function: https://play.golang.org/p/Opd10xBfZKu
What did you expect to see?
I expected the code not to compile, because the position len(slice)*2 can never be a valid position in the slice.
What did you see instead?
The code was compiled with bounds checks and always panics because of index out of range (the code produced is correct, but what I think is that it shouldn't be compiled in the first place - always results in panic).
Edit: after further testing, I noticed that the same happens for len(slice)+n, where n is a positive integer.

The spec (https://golang.org/ref/spec#Index_expressions) says

For a of slice type S:
if x is out of range at run time, a run-time panic occurs

So it should be a run-time panic, instead of compile-time error. I think this works as intended.

Yes, this is correct behavior.  We don't want to be a compilation error; the statement might never be executed.

authors: cherrymui ianlancetaylor marigonzes


text:
image/gif: bounds offset reversed when encoding

go 1.12.1 linux/amd64
What did you expect to see?
image shift not depending on saved format.
What did you see instead?
gif image shifted in the opposite direction to all other formats.
(several other image format generating libs tried and gif the only one in disagreement.)
demo example:
package main
import "image"
import "image/png"
import "image/jpeg"
import "image/gif"
import "os"

type offsetImage struct{
	image.Image
	Rect image.Rectangle
}

func (i offsetImage) Bounds() image.Rectangle{
	return i.Rect
}

func main() {
	img:=offsetImage{image.Rect(0,0,50,50),image.Rect(0,0,50,50) }
	w,_:= os.Create("1.png")
	png.Encode(w,img)
	w,_= os.Create("1.jpg")
	jpeg.Encode(w,img,nil)
	w,_= os.Create("1.gif")
	gif.Encode(w,img,nil)

	img.Rect=image.Rect(30,30,80,80)  // same size just offset

	w,_= os.Create("2.png")
	png.Encode(w,img)
	w,_= os.Create("2.jpg")
	jpeg.Encode(w,img,nil)
	w,_= os.Create("2.gif")
	gif.Encode(w,img,nil)

}

output
without shift:all white, all the same as expected

with shift:

(png image above not showing in github md?)

I was able to reproduce this, does seem that gif changes after the offset in a way that's different from the other image formats.
/cc @nigeltao for image/gif package

confirmed fixed. (with patch) demo now results in matching images and original code now works as expected. nice.

authors: katiehockman splace


text:
golang proxy request incorrect

when use proxy to request nginx, nginx can not be use correctly, After I rewrite some code in readRequest method then it can be succeed. The code I have already upload. So you can take a look. Waiting for your reply. Thank you
Now request.go code (from readRequest method):
req.Header = Header(mimeHeader)

	// RFC 7230, section 5.3: Must treat
	//	GET /index.html HTTP/1.1
	//	Host: www.google.com
	// and
	//	GET http://www.google.com/index.html HTTP/1.1
	//	Host: doesntmatter
	// the same. In the second case, any Host line is ignored.
	req.Host = req.URL.Host
	//if req.Host == "" {
	//	req.Host = req.Header.get("Host")
	//}
	if deleteHostHeader {
		delete(req.Header, "Host")
	}

So if I run the following code, ngxin won't report that it can't find "project error"
	remote, err := url.Parse(p.path)
			if err != nil {
				panic(err)
			}
			proxy := httputil.NewSingleHostReverseProxy(remote)
			req.URL.Path = Substring(urlPath.Path, StringLen(p.pattern), -1)

			if req.Method == "OPTIONS" {
				w.Header().Set("Access-Control-Allow-Origin", "*") //
				w.Header().Add("Access-Control-Allow-Headers", "content-type, accept, x-auth-token, X-Subject-Token,x-openstack-nova-api-version")
				w.Header().Add("Access-Control-Allow-Methods", "POST")
				w.Write([]byte("{\"test\":\"OPTIONS\"}"))
				return true
			}
			w.Header().Add("Access-Control-Allow-Origin", "*") //x-openstack-nova-api-version
			w.Header().Add("Access-Control-Allow-Headers", "content-type, accept, x-auth-token, X-Subject-Token,x-openstack-nova-api-version")
			w.Header().Add("Access-Control-Expose-Headers", "X-Subject-Token, x-openstack-nova-api-version,X-Auth-Token")
			w.Header().Add("Access-Control-Allow-Methods", "PUT,POST,GET,DELETE,OPTIONS")
			w.Header().Add("Access-Control-Allow-Credentials", "true")
			proxy.ServeHTTP(w, req)
		}

Looks like dup of #28168. Let me know if that's not correct.

authors: bradfitz uucckk


text:
x/build/cmd/coordinator: fails 'go test' vet check at head

This causes x/build CLs to fail TryBots. (Why don't we have a builder for x/build?)
https://storage.googleapis.com/go-build-log/6a9b22ce/linux-amd64_3c1e5e69.log
# golang.org/x/build/cmd/coordinator
cmd/coordinator/status.go:204:23: Sprintf format %s reads arg #1, but call has 0 args
FAIL	golang.org/x/build/cmd/coordinator [build failed]

CC @dmitshur, since I think he's planning to make some changes in cmd/coordinator anyway.

This is already resolved via CL 182017. (Existing CLs may need to be rebased to pick up the change.)

(Why don't we have a builder for x/build?)

I thought we had an issue about it, but I couldn't find one. So I added it to #30640 instead.
Historically, x/build did not have even trybots, that was a relatively recent addition. It wasn't possible before because of third-party packages in x/build, but that's no longer a problem (in module mode, which x/build uses for its trybots).

authors: bcmills dmitshur


text:
x/build/cmd/tip: _tipstatus endpoint error is not cleared on successful process start

At this moment, https://tip.golang.org/_tipstatus shows that it's currently running the latest commits from Go and x/website repos:
current=go=e94472a3/website=61d83737

However, it also shows an non-nil error:
error=builder.Init: checkout of go: reset: fatal: ambiguous argument '': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

exit status 128

I've noticed that it seems to be showing an error most of the time, yet tip.golang.org seems to be fine. Either it's showing a previous error, or something else. We should figure out why it's happening and make it so that it shows nil error most of the time. A non-nil error should represent a legitimate issue.
/cc @toothrot FYI.

The problem was that the error was never cleared, so it indeed always showed the "last error", not "current error". I've sent 2 CLs, one to improve precondition checking, the other to fix this issue.


text:
the tool to prioritization the test cases order by coverage rate

What version of Go are you using (go version)?

$ go version

Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (go env)?
go env Output
$ go env

What did you do?

What did you expect to see?
i want to find a tool to prioritization the test cases order by coverage rate.
What did you see instead?

Hi @gxs929.
This seem to be a question rather than an issue. I suggest posting it another medium that is better suited for questions, see https://golang.org/wiki/Questions. Compare to the issue tracker, it's more likely someone will see your question there and be able to reply to it.
Thanks.

Hi @gxs929.
This seem to be a question rather than an issue. I suggest posting it another medium that is better suited for questions, see https://golang.org/wiki/Questions. Compared to the issue tracker, it's more likely someone will see your question there and be able to answer it.
Thanks.
Aha，thanks for your suggest,cause i am a freshman to Github,there are lots of rules i dont know.


text:
reflect: DeepEqual panics when encountering a pointer cycle

What version of Go are you using (go version)?

$ go version
go version go1.12.7 darwin/amd64

Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/huandu/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/huandu/.gopkg"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.7/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.7/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/w5/42dq8w6922z_2p0v7ssfmmkr0000gn/T/go-build443570034=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/YJE6T3aprvt
package main

import (
	"fmt"
	"reflect"
)

func main() {
	nested := func() interface{} {
		nested := map[string]interface{}{}
		nested["foo"] = nested
		return nested
	}

	v1 := nested()
	v2 := nested()
	fmt.Println(reflect.DeepEqual(v1, v2))
}

What did you expect to see?
No stack overflow. reflect.DeepEqual should return true.
What did you see instead?
reflect.DeepEqual fails with stack overflow.

I think you mean it panics when it encounters a cycle. A nested map would be something like map[int]map[int]int.
I agree DeepEqual shouldn't reach a stack overflow in this case. If you want to work on a fix, that's welcome.

@av86743 please be nice; there's no need to tell new Go contributors "good luck with this".
This is documented, though:

As DeepEqual traverses the data values it may find a cycle. The second and subsequent times that DeepEqual compares two pointer values that have been compared before, it treats the values as equal rather than examining the values to which they point. This ensures that DeepEqual terminates.

So I think it's reasonable to follow this rule and also make this case terminate.

@av86743 please only leave comments if you're going to provide useful input or constructive criticism. For example, why is this not true? If you think it isn't, file a new issue. It seems to me like the case reported here can be fixed, following the rule already documented, so there's no reason to continue insisting in this thread.
Also, a second reminder about https://golang.org/conduct.

I have a quick fix on this. The root cause is simple: Value#Elem() method always returns a reflect.Value with RO flag, which causes CanSet returns false and shadows the visited map.
--- deepequal.old       2019-08-28 20:48:58.000000000 +0800
+++ deepequal.go        2019-08-28 21:26:13.000000000 +0800
@@ -34,17 +34,19 @@
        // We want to avoid putting more in the visited map than we need to.
        // For any possible reference cycle that might be encountered,
        // hard(t) needs to return true for at least one of the types in the cycle.
-       hard := func(k Kind) bool {
+       hard := func(k Kind, v1, v2 Value) bool {
                switch k {
-               case Map, Slice, Ptr, Interface:
+               case Map, Slice, Ptr:
                        return true
+               case Interface:
+                       return v1.CanSet() && v2.CanSet()
                }
                return false
        }

-       if v1.CanAddr() && v2.CanAddr() && hard(v1.Kind()) {
-               addr1 := unsafe.Pointer(v1.UnsafeAddr())
-               addr2 := unsafe.Pointer(v2.UnsafeAddr())
+       if hard(v1.Kind(), v1, v2) {
+               addr1 := v1.ptr
+               addr2 := v2.ptr
                if uintptr(addr1) > uintptr(addr2) {
                        // Canonicalize order to reduce number of entries in visited.
                        // Assumes non-moving garbage collector.

@huandu Great! Please send a CL (see https://tip.golang.org/doc/contribute.html) and we can take it from there.

authors: huandu mvdan


text:
context: add WithCancelSignal function

As previously discussed on issues such as #21521 and #16472, or on several other places handling POSIX signals through context seems to be somewhat useful (and a little bit hard to do correctly?), and I would like to propose a way to handle it by adding a new signal.WithContext function.
There's also an idea to improve the current approach to handling signals (see https://github.com/golang/go/issues/21521#issuecomment-337038699). I didn't give it a try yet, unfortunately. I only found the earlier proposals here after trying to write some code, so I decided to create this new issue anyway to share my idea.
People using some sort of it or talking about the subject:

https://github.com/henvic/ctxsignal (package I wrote a while ago)
https://twitter.com/dmitshur/status/1227777318162030592 (thread)
https://twitter.com/matryer/status/869096368039710720 (thread)
Make Ctrl+C cancel the context.Context (blog post)
https://github.com/oklog/run/commit/9c53bcd6fefd554246da88d55efcc283aff63659
https://github.com/FiloSottile/mostly-harmless/blob/22c6a9e08ad95b602b470652ad2d401d8750e264/covfefe/covfefe.go#L118-L139
https://github.com/shurcooL/home/blob/d0f5a32e4901b05ae9979b1a9e7b67caba549778/main.go#L61-L73
https://github.com/search?l=Go&p=1&q=context+signal&type=Repositories lists 18 other packages for the same thing.

Also please notice that this change would, unfortunately, require introducing packages time and context as dependencies for package os/signal.

Could you describe the new function in this issue, so that we don't have to figure it out from the CL?  Thanks.

The new function receives a parent context and a variadic number of signals that can be used to cancel this context. When a parent context is canceled or when one of the signals is handled, the context is canceled.
Example of handling cancelation of a slow process through SIGTERM and SIGINT:
func main() {
	ctx, cancel := signal.WithContext(context.Background(), syscall.SIGTERM, syscall.SIGINT)
	defer cancel()

	// ...
	slow.Run(ctx)
	cancel()

	// to verify if the slow function was terminated by a signal, we can use:
	var sigErr signal.ContextError
	if errors.As(ctx.Err(), &sigErr) {
		fmt.Printf("slow function terminated by signal %q\n")
	}
}

On another side, we have the example shown for https://golang.org/pkg/net/http/#Server.Shutdown that, in my opinion, would not be simplified by this.

Perhaps this should be a function of the context package instead, since it creates a context and it's pretty much all about contexts.
ctx, cancel := context.signal(context.Background(), syscall.SIGTERM, syscall.SIGINT)
Not sure it's the right idea at all, just throwing it out there.

If several packages handle same signal, and runtime does not handle those contexts equally, context.Signal won't work correctly, I think. If runtime does not handle them,  context.Signals is not best way . The os / signal package has the potential feeling for us that it is only used in applications, but the context package is used by everyone. So I'm thinking signal.WithContext is better since it make users known that we should do it in only one place. (But I'm also not sure this is right idea)

To be clear, the runtime does already broadcast signals to anyone who has signed up to hear about them, using signal.Notify. So this could be done as a separate package without worrying about other possible uses of os/signal.
context.WithCancelSignal seems like the clearest name, and it would appear right next to context.WithCancel in the docs.
/cc @Sajmani for thoughts

It sounds like maybe we have converged on context.WithCancelSignal. I've retitled.
Based on the discussion above, this seems like a likely accept.

Great! Thanks for the feedback. I'll try to send a new CL proposing an implementation for a context.WithCancelSignal command Monday.

Is there a plan for how this would be documented? Signals are process scoped, so any library using this feature could be problematic. And in a server I would normally catch signals at main level and propagate down, which can be easily/lazily done already by cancelling a single parent context. So using signals for local task cancellation seems only suited for interactive apps. If that's not too controversial, could it be made clear in the docs?

I pushed a new implementation to https://go-review.googlesource.com/c/go/+/219640/1

I note that currently according to go/build/deps_test.go context depends only on
{"errors", "internal/reflectlite", "sync", "sync/atomic", "time"}

This is going to add a dependency of context on os and os/signal, which pulls in a lot more.  I don't know that it matters much, but it seems worth  mentioning.  Is anybody concerned about the additional dependencies?

I was worried too, but if you look at the full lists it doesn't add much transitively. In the current tree:
% deps io os os/signal | sort -u
errors
internal/bytealg
internal/cpu
internal/oserror
internal/poll
internal/race
internal/reflectlite
internal/syscall/execenv
internal/syscall/unix
internal/testlog
io
os
runtime
runtime/internal/atomic
runtime/internal/math
runtime/internal/sys
sync
sync/atomic
syscall
time
unsafe
% deps context
errors
internal/bytealg
internal/cpu
internal/oserror
internal/race
internal/reflectlite
runtime
runtime/internal/atomic
runtime/internal/math
runtime/internal/sys
sync
sync/atomic
syscall
time
unsafe
%

The only addition would be io and os themselves, and three internals:internal/syscall/{unix,execenv} and internal/testlog.
I'm not saying I'm happy about this, but it's arguably OK.

If receiving the signal in goroutine and context.WithCancelSignal, I wonder how this work.
package main

import (
	"context"
	"os"
	"os/signal"
	"time"
)

func doSomething(ctx context.Context) {
	// ...
}

func main() {
	s := make(chan os.Signal, 1)
	signal.Notify(s, os.Interrupt)

	ctx := context.WithCancelSignal(s)
	doSomething(ctx)

	go func() {
		<-s
		println("interrupted")
	}()

	var b [1]byte
	os.Stdin.Read(b[:])
	time.Sleep(time.Second)
	println("exit")
}

One possibility to avoid adding a dependency on os and os/signal is to have the context package reach into the runtime package as the os/signal package already does.  One consideration about depending on the os/signal package is that the os/signal package has an init function that slightly changes how the runtime package handles signals.  But we might be able to get rid of that anyhow.

@mattn The proposed context.WithCancelSignal function takes a signal as an argument, not a channel.
But assuming you meant to pass os.Interrupt to context.WithCancelSignal, then on receipt of a signal both 1) the context would be canceled; 2) the signal would be sent on the channel s.

Oh, I see.

I'm worried about the number of changes in the signal package too. I'll take a look into the runtime package Friday to see if I understand @ianlancetaylor's idea and to see if I can come up with something.

I'm worried about the number of changes in the signal package too, and actually this is why it took me quite a while to send this. I'll take a look into the runtime package to see if I understand @ianlancetaylor's idea and to see if I can come up with something better.

FWIW, I think we can easily hook this into the runtime package directly (with an internal package if needed) and bypass os and os/signal, to keep the deps lite. The general trend is to avoid os where possible, and all this code only needs the runtime.
No change in consensus here, so accepted.

FWIW putting signal-related behaviour in the context package feels wrong to me. Currently the functionality in the context package is entirely about contexts themselves and time. It's a very clean and delimited API. However, signals are a very system-specific concept. The inclusion of syscall in the public API makes this quite clear. context.WithCancelSignal reads nicely as a name, I'll admit, but it feels like we're mixing concerns inappropriately to me, and I would much prefer the function to go into the signal package itself.
I think this may also be of concern to implementations of Go that run on limited infrastructure. For example in tinygo, the context package is currently available but the os/signal package is not. Adding this functionality to context would mean that only some of the context API could be implemented on that platform.

I agree. Not a domain expert but there seems to be a model break going on.

Thanks, @rogpeppe. Your concern sounds solid to me.
Perhaps this should indeed go to the signal package after all (or maybe it has its own issues besides feeling unnatural)?
Any thoughts? /cc @deadprogram

@rsc There is disagreement that arose after the proposal moved through likely accepted to accepted.  Should we move back to the proposal stage?

OK, moving proposal back to active (un-accepting).

@rogpeppe, I am a little confused about "the inclusion of syscall in the public API".
The public API would be
package context
func WithCancelSignal(ctx Context, ... os.Signal) (Context, func())

It mentions os, not syscall. It's true that calls might pass signals defined in syscall, but that will happen regardless.
It seems like the three choices would be:

syscall.SomethingContext - syscall imports context, argument can't be os.Signal, would need to be syscall.Signal, more OS-dependent than what was previously accepted.

os/signal.SomethingContext - os/signal imports context. argument could be os.Signal

os.SomethingContext - os imports context. argument could be os.Signal

It seemed cleaner to keep the context constructors in context. Is there a good argument for os or os/signal to start importing context and turn it around?

I know that "it feels wrong to me" isn't a great argument, but I agree with @rogpeppe - this felt wrong to me when it was accepted and it still feels wrong. context feels, to me, as a relatively pure abstraction and signal-handling like a relatively platform-dependent use-case.
To try a more tangible argument: Say, hypothetically, the opentracing API would make it into the stdllib. Would we put StartSpanFromContext into the context-package, just because it returns a Context? That seems far-fetched. Of course, we wouldn't put the opentracing API into the stdlib as is and whatever would make it might well have a more natural answer. But IMO it still illustrates that something might return a context, but still fit more naturally into another package.
Another "argument" is that signal.WithContext (or whatever) is naturally implemented as a pretty simple wrapper around context.WithCancel, so it at least has no need to share a namespace with the rest of the context package. Of course it also has no need to share a namespace with the signal-package, but at least it seems closer aligned with that concern IMO.
At the end of the day, both locations seem not fully satisfying to me and as a corollary I can also live with either. But if I was the one who's vague feelings it gets down to, I'd put it into signal as well :)

If we do decide that os/signal is the right place for this functionality, I think the name should be simply signal.Context.
package signal
func Context(ctx context.Context, ...os.Signal) (context.Context, context.CancelFunc)

I don't see how signal.WithContext makes sense, and any name that really describes the scenario gets pretty long, as in signal.CancelContextOnSignal.  I think signal.Context is clear enough.
@robpike Note that you suggested the context package above in https://github.com/golang/go/issues/37255#issuecomment-586801888 and then changed your mind in https://github.com/golang/go/issues/37255#issuecomment-611946231.  Which is fine, just wanted to double check.

@ianlancetaylor I didn't exactly change my mind. If you read those comments again, I think you'll see my position has been tentative at best all along.  @Merovius explains it well, I think. Context is and perhaps should remain an abstraction free of outside dependency.
I want to reiterate though that I am speaking only from the point of view of a general API designer, not as someone who is a heavy user of either the context or signal packages.

It mentions os, not syscall. It's true that calls might pass signals defined in syscall, but that will happen regardless.

Yes, sorry, my mistake - I read the syscall values in the examples and assumed the wrong signature. I think my point still stands though. The os package is quite large and system specific, which doesn't seem to fit with the small context API to me.

It seemed cleaner to keep the context constructors in context. Is there a good argument for os or os/signal to start importing context and turn it around?

I think so. Anyone that wishes to add any kind of custom cancellation to a context must provide their own constructor, and I don't see why signal-based cancellation should be any different.
If it's inefficient to do so, then perhaps that's something more general that could be addressed in the context package.
FWIW I support @ianlancetaylor's suggestion of a signal.Context function.

@ianlancetaylor @bradfitz and I bikeshedded the name for a while. This is the best we came up with:
package signal
func NotifyContext(parent context.Context, signals ...os.Signal) (ctx context.Context, cancel context.CancelFunc)

NotifyContext returns a copy of the parent context that is marked done
(it's Done channel is closed) when one of the listed signals arrives,
when the returned cancel function is called, or when the parent context's
Done channel is closed, whichever happens first.

Canceling this context releases resources associated with it, so code should
call cancel as soon as the operations running in this Context complete.

Thoughts?

From the perspective of a user, this sounds good and very useful to me to write cleaner code.
Also, what about an error type such as the ContextError I implemented in patchset 1 https://go-review.googlesource.com/c/go/+/219640/1?

// ContextError is the error returned by Context.Err when the context is
// canceled by an operating system signal.
type ContextError struct {
	sig os.Signal
}

func (e ContextError) Error() string {
	return "context canceled after receiving signal " + e.sig.String()
}

// Signal that canceled the context.
func (e ContextError) Signal() os.Signal { return e.sig }

The idea is to make it possible to identify what signal canceled it. This is useful for deciding what to do next. For example, you might want to delegate handling your signals to a function and then differentiate what is happening there, like:
SIGHUP: configuration file reload.
SIGTERM, SIGINT: stop handling new HTTP requests and exit applications gracefully.

Canceling this context releases resources associated with it, so code should
call cancel as soon as the operations running in this Context complete.

Does canceling the context also (potentially) change the signal behavior of the program?
If so, does that signal behavior only change when cancel is called, or can that also happen implicitly when the parent context becomes done? (Personally, I would prefer that it only occur when cancel is called.)

I'm not necessarily opposed to context.SignalError, but I think that if you want to make decisions based on which signal was received then you shouldn't be using signal.NotifyContext.  You should be using context.WithCancel and signal.Notify, and canceling the context when receiving a value from the signal.Notify channel.  We already have a mechanism for handling signals; the point of this change is to provide a convenient way to cancel a context on receipt of a signal.  If you have to do different things based on different signals, we already have mechanisms for that.

Ouch, I hadn't thought about changes in signal handling behavior.  I agree that the only possible choice here is that the signal handling behavior only changes when or if the cancel function is explicitly called.  Otherwise the wrong thing would happen if the program received two SIGHUP signals in a row.

Personally I will probably not use this new function.
I'm in favor of not adding this to the stdlib.
It's very simple to write your own implementation in your app and the need may be different in each app.
I've already built a package that I'm using at my company, and I don't think it's compatible with your proposal.
Here what it is doing:

the first time it receives a signal, it cancels the context (without unregistering the signal watcher)
the second time it receives the signal, it exits the app
the caller must explicitly unregister the signal watcher

Here is the code if someone is interested:
// Package ctxsignal provides a bridge for context and POSIX signals.
package ctxsignal

import (
	"context"
	"log"
	"os"
	"os/signal"
	"sync"
	"syscall"

	"github.com/xxx/yyy/goroutine"
)

// RegisterCancel registers a listener for the SIGINT/SIGTERM signals.
// The first time that the signal is received, the context is canceled.
// The second time, the program exits with status 0.
func RegisterCancel(parent context.Context) (ctx context.Context, unregister func()) {
	ctx, cancel := context.WithCancel(parent)
	c := make(chan os.Signal, 1)
	signal.Notify(c, syscall.SIGINT, syscall.SIGTERM)
	waitWatchSignal := goroutine.Go(func() {
		watchSignal(c, cancel)
	})
	unregister = newUnregisterFunc(c, waitWatchSignal)
	return ctx, unregister
}

func watchSignal(c <-chan os.Signal, cancel func()) {
	sig, ok := <-c
	if !ok {
		return
	}
	log.Printf("%q signal received, context canceled. Send it again to exit the program immediately.", sig)
	cancel()
	_, ok = <-c
	if !ok {
		return
	}
	log.Println("Signal received twice, exit now.")
	os.Exit(0)
}

func newUnregisterFunc(c chan os.Signal, waitWatchSignal func()) func() {
	var once sync.Once
	return func() {
		once.Do(func() {
			signal.Stop(c)
			close(c)
		})
		waitWatchSignal()
	}
}

(waitWatchSignal is a function that blocks until the goroutine is finished).

Not an expert in signal handling by any means, but...
Why would it be bad to change the signal handling behavior when the parent context is canceled? Wouldn't also be similarly bad if it doesn't (especially if there is a way to check what signal caused the context to end)? It's not clear to me what an example would be. Instead, shouldn't NotifyContext be used first when this happens?
Another thing to worry: if you want to use this for something like handling configuration reload through SIGHUP, it might be fine if you lose a signal (if you replace the context immediately to keep this going on - I'm not sure if this would be good btw). However, if you're listening to, say, SIGTERM, if a second SIGTERM arrives before you called signal.Notify again, your process might terminate before it should.

The idea is to make it possible to identify what signal canceled it.

There is already a pretty strong precedent that ctx.Err() is exactly either context.Canceled or context.DeadlineExceeded. Rather than deviating from that precedent, it might be less intrusive to encode the signal in the Value method and provide a top-level accessor for it:
package signal

// ContextSignal reports which (if any) signal caused ctx to be canceled.
//
// If ctx was not canceled due to a signal, ContextSignal returns (nil, false).
func ContextSignal(context.Context) (os.Signal, bool) {
	sig, ok := ctx.Value(signalContextKey{}).(os.Signal)
	return sig, ok
}

@bcmills, I like it.

@bcmills, If somewhere within a call stack, I had an operation, that accepted the context and reacted to its cancellation, I don't think I would check something related to the signal in this operation, but rather returned ctx.Err(). After this error bubbled up to someone who "cares" about the signals, it would

check if the error wasn't nil
check if the context was cancelled
check if there is a signal in the context's value

Is that how ContextSignal expected to be used?

To fix the double-signal problem, it sounds like we have to deviate from the usual context.WithCancel to require calling the cancellation function if you want to stop the captured signal behavior. That is, the docs would change to:
package signal
func NotifyContext(parent context.Context, signals ...os.Signal) (ctx context.Context, stop context.CancelFunc)

NotifyContext returns a copy of the parent context that is marked done
(it's Done channel is closed) when one of the listed signals arrives,
when the returned stop function is called, or when the parent context's
Done channel is closed, whichever happens first.

The stop function unregisters the signal behavior, which, like signal.Reset,
may restore the default behavior for a given signal. For example, the default
behavior of a Go program receiving os.Interrupt is to exit. Calling
NotifyContext(parent, os.Interrupt) will change the behavior to cancel
the returned context. Future interrupts received will not trigger the default
(exit) behavior until the returned stop function is called.

The stop function releases resources associated with it, so code should
call stop as soon as the operations running in this Context complete and
signals no longer need to be diverted to the context.

It remains unclear that we really need the signal details. For that, it's easy enough to write the simple code that handles the signal, records it, and cancels a context. That is, NotifyContext is meant to be more a convenience than a complete Swiss army knife. We can always add it later if we need to, but it probably makes sense to start without it.
Thoughts?

It makes sense to me. I liked the idea of calling the cancel function stop to differentiate things.

Based on the discussion above, it seems like we've reconverged on signal.NotifyContext, documented in https://github.com/golang/go/issues/37255#issuecomment-621357594.
This seems like a likely accept (take two).

No change in consensus (yet?) so accepted (again).

@henvic Were you planning to adapt your CL (or a new one) based on the change to os/signal.NotifyContext? I'd be happy to help however I can, would be great to get this in for 1.16.

@danp, thank you for the help!
Hello @ianlancetaylor, might you please take a second look at the updated CL? I have sent a new patchset. Thank you!
https://go-review.googlesource.com/c/go/+/219640

authors: Merovius bcmills danp henvic ianlancetaylor mattn narqo pierrre robpike rogpeppe rsc undeconstructed


text:
Go2 returning error automatically without manually checking it

I am working on a complex database transaction with 500+ lines of code inside and most of the functions are returning errors. The error handling in go is totally annoying in my case because no matter which error I get, the entire code is just not committing the database transaction by reverting any in-memory changes. So after writing 1000+ additional lines of code of if err = func1(); err !=  nil { return } only for checking errors I decided to switch to panics which removes 500+ lines of the additional code, but I do still have many error checks... Why don't we have a simple error checking operator like output := func1() ? or output, err := func1() or return. The golang is great but the error handling is simply terrible tbh.

This has been discussed countless times and you've ignored the issue/proposal template. Closing.

Of course I have read some of them. I just wanted to post my prost against the error handling of the golang. I even gave a concrete example where the golang error handling doesn't make any sense. In transactional databases.

The issue tracker is not the place to complain about error handling. Locking.

authors: ibudisteanu mdlayher


text:
cmd/cover: inconsistent NumStmt with prometheus

What version of Go are you using (go version)?

$ go version
go version go1.16.2 linux/amd64

Does this issue reproduce with the latest release?
It is indeed the latest release
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/.go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/.go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.2"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2943966716=/tmp/go-build -gno-record-gcc-switches"

What did you do?
git clone https://github.com/prometheus/prometheus
cd prometheus/promql/parser
go test -coverprofile=wrong.out
go tool cover -html wrong.out
What did you expect to see?
Get the HTML coverage report
What did you see instead?
Error message cover: inconsistent NumStmt: changed from 2 to 1
The wrong.out contains indeed (running grep generated_parser.y.go wrong.out | grep 178
github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:178.0,180.0 2 1
github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:179.0,178.0 2 1
github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:176.0,178.0 1 1
github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:178.0,180.0 1 1

I found this by modifying the covermessage with adding %#+v about other fields

It still does not work on my laptop.

OS: macOS Catalina 10.15.5 19F101 x86_64 / Intel Chip
Go version: 1.18.3

Any update on this issue?

I encountered the same problem with the latest Go 1.20. Unfortunately, it's in a proprietary software, but also applies to a generated file -- in our case by Ragel. I'll try to find reproducer for this.

friendly ping @golang/runtime ?

@neild the line directive strikes again it seems (messing up line numbers between the generated file and the generating file)

It seems that //line directives inserted into generated parser by goyacc result in invalid entries in coverprofile.
The workaround could be to disable line directives via -l flag, see https://github.com/zalando/skipper/pull/2450.

authors: AlexanderYastrebov catenacyber wojciech-sneller yufeiminds


text:
proposal: add generics label for generics-related issues

Similar to the modules label, a generics label would make keeping track of the multiple generics-related proposals that are floating around at the moment, as well as any new ones that are filed, a bit easier. An umbrella issue for generics proposals could also work.

We already have such a label: https://github.com/golang/go/labels/generics .
I just added it to #43651, #45346, #45458.  Sorry for not doing that earlier.

authors: DeedleFake ianlancetaylor


text:
syscall: in TestStdioAreInheritable, powershell on arm64/arm cannot load arm64/arm binaries because it is an intel process

ARM builds of Windows are still shipping an Intel powershell, which means we can't test arm shared libraries using powershell as a loader. This bug is to track that, if it ever changes, so we can remove the test skippage.

Is it possible to rewrite the test to not depend on powershell?

Is it possible to rewrite the test to not depend on powershell?

When I introduced TestStdioAreInheritable in CL 316269, I just adopted repro provided at https://github.com/golang/go/issues/44876#issue-825396212. After successfully built TestStdioAreInheritable, I tried to do something similar in C (compiled with GCC). But I could not do it.
I don't remember how much effort I put into conversion of TestStdioAreInheritable into C, but that was significant enough. I don't plan to work on this now. I also don't have any notes of what I tried.
Alex

I don't remember how much effort I put into conversion of TestStdioAreInheritable into C, but that was significant enough. I don't plan to work on this now. I also don't have any notes of what I tried.

Should we just remove the test? Is it particularly useful?

Should we just remove the test?

Why? What is the benefit?

Is it particularly useful?

The test verifies that #44876 stays fixed. I think it is useful.
Alex

authors: alexbrainman cherrymui zx2c4


text:
x/mobile: bind tests (bind and cmd/gobind) are broken

https://storage.googleapis.com/go-build-log/fdab5be1/linux-amd64-androidemu_5f1949ec.log
ok  	golang.org/x/mobile/bind/seq	0.026s
--- FAIL: TestDocs (0.46s)
    --- FAIL: TestDocs/Modules (0.33s)
        gobind_test.go:173: gobind -lang java failed: exit status 1: // Code generated by gobind. DO NOT EDIT.
     ...
            "golang.org/x/mobile/bind" is not found; run go get golang.org/x/mobile/bind: [-: missing go.sum entry for module providing package golang.org/x/mobile/bind; to add:
            	go mod download golang.org/x/mobile]
        gobind_test.go:173: gobind -lang objc failed: exit status 1: // Objective-C API for talking to example.com/doctest Go package.
     ...
            unable to import bind/objc: [-: missing go.sum entry for module providing package golang.org/x/mobile/bind/objc; to add:
            	go mod download golang.org/x/mobile]
FAIL
FAIL	golang.org/x/mobile/cmd/gobind	80.625s
--- FAIL: TestBindWithGoModules (19.43s)
    --- FAIL: TestBindWithGoModules/android (17.07s)
        --- FAIL: TestBindWithGoModules/android/Absolute_Path (10.88s)
            bind_test.go:319: gomobile bind failed: exit status 1
                /workdir/tmp/gomobile-test439913593/gomobile: go build -buildmode=c-shared -o=/workdir/tmp/gomobile-work-721723007/android/src/main/jniLibs/armeabi-v7a/libgojni.so ./gobind failed: exit status 1
                go: github.com/BurntSushi/xgb@v0.0.0-20160522181843-27f122750802: missing go.sum entry; to add it:
                	go mod download github.com/BurntSushi/xgb
                go: github.com/BurntSushi/xgb@v0.0.0-20160522181843-27f122750802: missing go.sum entry; to add it:
                	go mod download github.com/BurntSushi/xgb

        --- FAIL: TestBindWithGoModules/android/Relative_Path (6.18s)
            bind_test.go:319: gomobile bind failed: exit status 1
                /workdir/tmp/gomobile-test439913593/gomobile: go build -buildmode=c-shared -o=/workdir/tmp/gomobile-work-3749682380/android/src/main/jniLibs/armeabi-v7a/libgojni.so ./gobind failed: exit status 1
                go: github.com/BurntSushi/xgb@v0.0.0-20160522181843-27f122750802: missing go.sum entry; to add it:
                	go mod download github.com/BurntSushi/xgb
                go: github.com/BurntSushi/xgb@v0.0.0-20160522181843-27f122750802: missing go.sum entry; to add it:
                	go mod download github.com/BurntSushi/xgb

All about missing go.sum entries. I think this is related to gobind generates go.mod file based on the binding source, but doesn't generate go.sum file. I wonder if simply running go mod tidy before building can fix the issue.
cc @hajimehoshi

I thought @jayconrod fixed this in CL 323149. Is it still failing at head?

authors: bcmills hyangah


text:
cmd/compile: incorrect inlining of function swapping two funcs

What version of Go are you using (go version)?

$ go version
go version go1.19.7 windows/amd64

Does this issue reproduce with the latest release?
It does with 1.19; not with 1.20.
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\telic\AppData\Local\go-build
set GOENV=C:\Users\telic\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\telic\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\telic\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.19.7
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\F2\Texter\go.mod
set GOWORK=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\Users\telic\AppData\Local\Temp\go-build2840717194=/tmp/go-build -gno-record-gcc-switches

What did you do?
See test in playground.
By uncommenting the commented-out line in test() (introducing two blank variables) you can get the test to pass, presumably by preventing inlining.
What did you expect to see?
Output "01", indicating first the first, then the second func getting called.
What did you see instead?
Output "00", indicating the first func instead got called twice.

According to the decompilation at https://godbolt.org/z/hacWM8Ejd
The bug is introduced with 1.16, 1.15 still has both functions, starting with 1.16 to 1.19 the inlined b function is optimized away.

Fixed for Go1.20 here: 54632

Duplicate of #54632

This reproduces against 1.19.
Shouldn't the fix be backported?

Curious: Is a fix for Go 1.19.x planned?

@gopherbot Please open backport issue for 1.19
Reportedly this issue occurs in 1.19 as well.

authors: Thoro christophberger fabe-xx ianlancetaylor ncruces seankhliao


text:
How often does Go GC after the memory exceeds the GOMEMLIMIT limit?

My system triggers OOM when it exceeds 2G. I set the value of GOMEMLIMIT to 1.5G. However, when the memory exceeds 1.5 GB, the GO frequently performs GC, which affects the system performance. I would like to know how the frequency of GC is determined when the memory exceeds the value set by the GOMEMLIMIT

1hz

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.
Please see: https://go.dev/wiki/Questions


text:
goproxy-forward-util-succuss-or-last

GOPROXY on the master node promises developers this：
For each request, the go command tries each proxy in sequence,
only moving to the next if the current proxy returns a 404 or 410 HTTP response.

But after a rough look at the code, I think the code on the current master node may not work as expected.
What's more, I think this logic is not in line with the developer's expectations, GOPROXY should try all the proxies in its list until one of them does not return an error.
Even if any of the proxies in the GOPROXY list have an error, then golang’s attempt to other proxies should not be interrupted.
The fact is that most goproxy providers (such as athens, goproxy.io) return 500 when the server fails to pull locally.
More about it: https://github.com/storyicon/gos/blob/master/docs/on-goproxy.md

@bcmills  Now I think that only when an proxy in the GOPROXY list returns 403/200, golang's attempt on the next proxy should be interrupted. Do you have any ideas?

The behavior introduced by this change does not maintain the privacy-preserving characteristics of GOPROXY as designed — see #31913 for more detail.

source: gharchive/pull-request

authors: bcmills storyicon


text:
Adding custom emulators

Is it possible to reference a custom emulator in a JS middleware fashion?
I guess it is always possible to clone and add an emulator class, but it'd be nice to add the flexibility to post-process the code according to anyone's need

Interesting idea, could you be more specific at which point you would want to add your specific emulator and what use case you are looking at? The emulators are restructuring the response json - what keeps you from doing that in your application code?

In my case, modifying the response is exactly what I want. But I guess in some cases you might want to preprocess the data too. For example lowercase all user entries (may be this can be done with a pipeline component). That's what I like with the middleware approach found in many javascript frameworks (e.g express.js). You can chain as many as you want so you have a lot of flexibility to transform incoming and outgoing data

@znat would you say this is still relevant?

@akelad Probably not.

authors: akelad twerkmeister znat


text:
Move worker service implementation from golem-worker-service to golem-worker-service-base

It involves the following

Adding authorisation headers to the metadata for GRPC requests
Difference between worker service in cloud and OSS is, we have account authorisation in every method

Same as #282

repo: golemcloud/golem


text:
Rib Robustness: Remove support of out interpolations from Rib Syntax

The original description of this ticket was testing a specific syntax for which type inference failed.
However, after investigating more, we found that type_inference was actually correct and the issue was related to parser,
where it tries to incorporate the support of interpolations at the outer level.
As part of this ticket, we need to ensure we remove the support of outer interpolations from Rib Grammar, and remove any hacks around it.
Make sure we have backward compatibility with the help of tests, to support those older expressions that may be wrapped in ${..}. This goes for both persistent layer as well as API layer. Make sure these extra fuctions are well commented, that it is only to support backward compatibility.
Update the existing tests to focus on un-interpolated expressions for better maintainability.

There might be doc updates required here.

authors: afsalthaj jdegoes


text:
Long URL results in "Payload Too Large"

This might be in companion to the grimoire container however:
SvelteKitError: Content-length of 694059 exceeds limit of 524288 bytes.
    at Object.start (file:///app/build/handler.js:984:19)
    at setupReadableStreamDefaultController (node:internal/webstreams/readablestream:2437:23)
    at setupReadableStreamDefaultControllerFromSource (node:internal/webstreams/readablestream:2470:3)
    at new ReadableStream (node:internal/webstreams/readablestream:290:7)
    at get_raw_body (file:///app/build/handler.js:973:9)
    at getRequest (file:///app/build/handler.js:1052:7)
    at Array.ssr (file:///app/build/handler.js:1230:19)
    at handle (file:///app/build/handler.js:1300:23)
    at file:///app/build/handler.js:1300:40
    at Array.<anonymous> (file:///app/build/handler.js:1219:4) {
  status: 413,
  text: 'Payload Too Large'
}

Reproduce:

load up: https://www.homedepot.com/p/CyberPower-500VA-120-Volt-6-Outlet-Rackmount-UPS-OR500LCDRM1U/311903186
Initiate grimoire companion
Try to add bookmark

Hello @axsddlr! Thank you for reporting it. Although, I have tested this extension with a considerable list of URL's, there's always some possibility I didn't catch every edge-case.
I will look into it!

Hello @axsddlr! Thank you for reporting it. Although, I have tested this extension with a considerable list of URL's, there's always some possibility I didn't catch every edge-case.
It appears that the HTML content fetched from the browser is too big for the default limit. I will look how to handle it properly!

To be fair, adding it manually in grimoire is fine. Thats why I suspect its specific to the extension.

From what I can find, the default value for BODY_SIZE_LIMIT of SvelteKit is to blame (only 512kb, source).
Because Grimoire loads all .env variables during start, you should be able to resolve this issue by adding BODY_SIZE_LIMIT=5000000 to it.

@goniszewski that worked!

authors: axsddlr goniszewski

repo: goniszewski/grimoire-web-extension

license: MIT


text:
ルーレットイベント

選択肢たち: string[] を与えて、そのうち 1 個をルーレットで選ぶイベント: EventRoulette extends GameEvent<string>
result() は選ばれた選択肢を返すように

#63

repo: gonoji/momotech


text:
Compiler fails to compile int8 quantized TFLite Model

Description
I want to convert a int8 quantized tflite model:
converter.optimizations = [tf.lite.Optimize.DEFAULT]
def representative_dataset_gen():
  # Get sample input data as a numpy array in a method of your choosing.
  for data in dataset:
    # not sure why, for single input data use [data] for multiple inputs just data...
    yield [data]
converter.representative_dataset = representative_dataset_gen
converter.target_spec.supported_ops = [tf.lite.OpsSet.TFLITE_BUILTINS_INT8]
converter.inference_input_type = tf.uint8
converter.inference_output_type = tf.uint8
converter.experimental_new_converter = True
tflite_model = converter.convert()

I can load the quantized tflite model without issues and run inference with it. But when I want to convert it the edgetpu compiler fails with many of these errors:
Edge TPU Compiler version 16.0.384591198
Searching for valid delegate with step 1
Try to compile segment with 136 ops
Started a compilation timeout timer of 180 seconds.
ERROR: :344 no_integer_overflow_from_quantization was not true.
ERROR: Node number 31 (CONV_2D) failed to prepare.

Compilation failed: Model failed in Tflite interpreter. Please ensure model can be loaded/run in Tflite interpreter.
Compilation child process completed within timeout period.
Compilation failed!

I assume this means that somewhere in the quantized model there are integers outside of int8 range. But that should not happen with the quantized tflite model, right?
model_quant.tflite.zip
And thanks for adding more meaningful error messages with the new compiler update!
Click to expand!
Issue Type
Bug, Support
Operating System
Linux
Coral Device
No response
Other Devices
No response
Programming Language
Python 3.7
Relevant Log Output
No response

@j-o-d-o can you try to export the saved_model without bathnorm for layer 31 ?
model.html.tar.gz

@hjonnala that doesnt seem to help. Same error message for the next Layer:
ERROR: :344 no_integer_overflow_from_quantization was not true.
ERROR: Node number 37 (CONV_2D) failed to prepare.

I mean, I know what the error is trying to tell me. What I do not understand is why they are happening and how I would look into this issue. I am a bit surprised that all batchnorm and bias layers are quantized with int32 despite setting converter.target_spec.supported_ops = [tf.lite.OpsSet.TFLITE_BUILTINS_INT8] and it seems that might be the issue when trying to compile the edgetpu model.

Any updates on this one? I am kind of stuck on here :(

Have you exported the model with fixed tensor shape? only fixed shape tensors or models will work on edgetpu.
I have tried to quantize the model the saved model and I am getting
ERROR: Attempting to use a delegate that only supports static-sized tensors with a graph that has dynamic-sized tensors..
edgetpu_453.ipynb.tar.gz

Hi, any updates on this? I'm having the sampe problem. Compiler setup is:
converter.target_spec.supported_ops = [tf.lite.OpsSet.TFLITE_BUILTINS_INT8]
converter.inference_input_type = tf.uint8
converter.inference_output_type = tf.uint8
converter.optimizations = [tf.lite.Optimize.DEFAULT]

and compiling with !edgetpu_compiler -a -s model.tflite yields
Edge TPU Compiler version 16.0.384591198
Started a compilation timeout timer of 180 seconds.
ERROR: :344 no_integer_overflow_from_quantization was not true.
ERROR: Node number 15 (DEPTHWISE_CONV_2D) failed to prepare.

Compilation failed: Model failed in Tflite interpreter. Please ensure model can be loaded/run in Tflite interpreter.
Compilation child process completed within timeout period.
Compilation failed!

That's puzzling, since the same model compiles fine with compiler version 15.0.340273435. Here's the tflite model in question.

@hjonnala The original issue I posted is still present. My last message is only a workaround. I don't think it is a viable solution to this issue to tell developers that they must set an unsupported flag (_experimental_new_quantizer).
Also, if I were to develop this compiler I would really want to know why it fails with the default quantization and works with the experimental_new_quantizer.

@xadrianzetx yes, its due to  depth wise conv layer limitation(Tensor has unsupported rank (up to 3 innermost dimensions mapped), those ops  are mapped to CPU.

Fee free to reopen if you have questions.

I have a similar problem with compilation. Not sure if it should be part of this thread or a separate thread, but it seems to be an issue with weight conversion to int8
My model compiles when I create it but don't load my Tensorflow weights: If I create the model, don't train or load weights, quantize and compile with the same settings as in this thread, things work.
When I load pre-trained weights, and repeat the procedure, it doesn't, fails without an error message
edgetpu_compiler -a -s -t 1000 {save_path} returns:
'Edge TPU Compiler version 16.0.384591198',  'Started a compilation timeout timer of 1000 seconds.',  'Compilation child process completed within timeout period.',  'Compilation failed! '
When pre-trained weights are not loaded, the log file shows the following layers:
`Edge TPU Compiler version 16.0.384591198
Input: /home/ubuntu/network_quant.tflite
Output: network_quant_edgetpu.tflite
Operator                       Count      Status
CONCATENATION                  3          Mapped to Edge TPU
SOFTMAX                        1          Mapped to Edge TPU
RESHAPE                        2          Mapped to Edge TPU
ADD                            19         Mapped to Edge TPU
QUANTIZE                       2          Mapped to Edge TPU
TRANSPOSE_CONV                 3          Mapped to Edge TPU
CONV_2D                        59         Mapped to Edge TPU
`
Software versions:  Tensorflow 2.8.0, Python 3.9.12
 converter.optimizations = [tf.lite.Optimize.DEFAULT]   converter.representative_dataset = representative_data_gen   converter.target_spec.supported_ops = [tf.lite.OpsSet.TFLITE_BUILTINS_INT8]   converter.inference_input_type = tf.uint8   converter.inference_output_type = tf.uint8

authors: aeon0 estebanvalles hjonnala j-o-d-o xadrianzetx

repo: google-coral/edgetpu

license_source: bigquery


text:
prove of the simple problem of Fig. 1

I have tried the simple problem of Fig. 1, and it can be proved.
Is there anything wrong?
test
a b c = eq2_triangle a b c ? eqangle b c b a c a c b

where eq2_triangle is defined as:
eq2_triangle a b c
c : a b
 =
a : ; b : ; c : cong a b a c
eq2_triangle

def sketch_eq2_triangle(args: tuple[gm.Point, ...]) -> tuple[Point, ...]:
  b = Point(0.0, 0.0)
  c = Point(np.random.uniform(0.5, 1.0), 0)
  a = Point((b.x + c.x) / 2, np.random.uniform(0.5, 10.0))
  return a, b, c

If you add the eq2_triangle definition to defs.txt without adding an extra breakline (\n) at the end you get this:
Traceback (most recent call last):
  File "/home/user/anaconda3/envs/alphageo/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/user/anaconda3/envs/alphageo/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/mnt/array50tb/projects/alphageometry/alphageometry.py", line 645, in <module>
    app.run(main)
  File "/home/user/anaconda3/envs/alphageo/lib/python3.10/site-packages/absl/app.py", line 308, in run
    _run_main(main, args)
  File "/home/user/anaconda3/envs/alphageo/lib/python3.10/site-packages/absl/app.py", line 254, in _run_main
    sys.exit(main(argv))
  File "/mnt/array50tb/projects/alphageometry/alphageometry.py", line 603, in main
    DEFINITIONS = pr.Definition.from_txt_file(_DEFS_FILE.value, to_dict=True)
  File "/mnt/array50tb/projects/alphageometry/problem.py", line 304, in from_txt_file
    return cls.from_string(lines, to_dict)
  File "/mnt/array50tb/projects/alphageometry/problem.py", line 309, in from_string
    data = [cls.from_txt('\n'.join(group)) for group in reshape(lines, 6)]
  File "/mnt/array50tb/projects/alphageometry/problem.py", line 34, in reshape
    assert len(l) % n == 0
AssertionError

When solving that and adding the sketch_eq2_triangle function to numericals.py I get the following output which is valid:
==========================
 * From theorem premises:
A B C : Points
AB = AC [00]

 * Auxiliary Constructions:
: Points

 * Proof steps:
001. AB = AC [00] ⇒  ∠ABC = ∠BCA
==========================

My point is that, with the above codes, it can be proved successfully. But, in Fig 1, it is stated that it can be proved without the auxiliary construction provided by LLM.
There must be something wrong, either my code, or Fig 1.

authors: Jerry-Master foldl

repo: google-deepmind/alphageometry


text:
feat: Add support for Vertex AI - WIP

Fixes #1

Replaced by https://github.com/google-deepmind/onetwo/commit/d6fce05f78b26ff9ec9fc316de8c57e9c08acb56

repo: google-deepmind/onetwo


text:
Fix bridge observation tensor

Fix the bridge observation tensor in #1167.

Thanks @zizhang-qiu !

Can you regenerate the playthroughs?
(run this script)

authors: lanctot zizhang-qiu


text:
Android Basics issue:

URL of codelab
https://developer.android.com/codelabs/android-basics-kotlin-affirmations-test-lists-and-adapters?continue=https%3A%2F%2Fdeveloper.android.com%2Fcourses%2Fpathways%2Fandroid-basics-kotlin-unit-2-pathway-3%23codelab-https%3A%2F%2Fdeveloper.android.com%2Fcodelabs%2Fandroid-basics-kotlin-affirmations-test-lists-and-adapters#6
In which task and step of the codelab can this issue be found?
Test Lists and Adapters - Step 7
Describe the problem
I do not know, where or how to paste the code provided in Step7 - it always shows errors. In the solution code in step 11 I do not find the file  AffirmationListTests.kt, so I can't look up my mistake.
Steps to reproduce?

Go to...
Click on...
See error...

Versions
What version of Android Studio are you using?
Additional information
Include screenshots if they would be useful in clarifying the problem.

@hermanngerdes
Thank you for reaching out to us and sharing your concerns. Hope the screenshot below (with the path and code snippet provided in the step 7 of the codelab) will help you to find the required file:

Happy testing!

authors: hermanngerdes osuleymanova


text:
Run on Multiple Images

Description of the feature request:
I cant run on multiple images as i am trying to run it on many images and store there output but it is saying
{
"image": "800040_2.pdf_4.jpg",
"description": "I am sorry, I cannot access or process any external files, including images. Therefore, I am unable to extract information from the provided image. \n\nTo assist you, I need the text content of the menu itself. If you can copy and paste the text here, I can help you process it and return the data in the JSON format you requested. \n"
}
What problem are you trying to solve with this feature?
But when i run on single images it works  please let me know what i have done wrong
Any other information you'd like to share?
import os
import json
import google.generativeai as genai
import google.generativeai as genai
from google.colab import userdata
GOOGLE_API_KEY=userdata.get('secretName')
genai.configure(api_key=GOOGLE_API_KEY)
model = genai.GenerativeModel('gemini-1.5-flash')
image_directory = '/content/unzipped_files/New folder/'
for filename in os.listdir(image_directory):
if filename.lower().endswith(('.png', '.jpg', '.jpeg')):
img_path = os.path.join(image_directory, filename)
    # Define your prompt
    prompt = """This image contains the restaurant menu we want to extract the dish name, section , price, description. section will be available at top only if not give blank section. Return output in json format:
                    {section: section , dish name: dish name, price:price, description:description}"""

    # Generate content using the model
    response = model.generate_content([prompt, img_path])  # Pass prompt and image path

    # Create a dictionary for this image
    image_data = {
        'image': filename,
        'description': response.text  # Access the text from the response
    }

    # Save the dictionary as a JSON file
    json_filename = os.path.splitext(filename)[0] + '.json'  # Remove extension and add .json
    json_path = os.path.join(image_directory, json_filename)
    with open(json_path, 'w') as f:
        json.dump(image_data, f, indent=4)

print("JSON files created successfully!")

@sauravsinhaa, Thank you reporting this issue.
This repository is for issues related to Cookbook guides and examples for Gemini API. The output is generated by the Gemini API and for issues related to Gemini API, we would suggest you to use "Send Feedback" option in Gemini docs. Ref: Screenshot below. You can also post this issue on Discourse forum.

repo: google-gemini/cookbook


text:
❓ Pretraining Details

Hi there,
I have several questions regarding pre-training of TAPAS:

max_position_embeddings: Set to 1024 for TAPAS. However, the original BERT models used 512. In the paper you state that you initialize your model from a text pre-trained BERT model. Does that mean you (1) don't initialize the position embedding, (2) copy the position embedding vector two times, (3) initialize the remaining embeddings randomly, or (4) use a custom pre-trained BERT model with max_position_embddeddings=1024

max_seq_length: Set to 128 in the pre-training example in the README. In the paper you state two different numbers. Just wanted to make sure, which max_seq_length was used in the end:

For pre-training to be efficient, we restrict our word piece sequence length to a certain budget (e.g., we use 128 in our final experiments)

We run both pre-training and fine-tuning on a setup of 32 Cloud TPU v3 cores with maximum sequence length 512.

Pre-training/fine-tuning: e.g. for the model tapas_wtq_wikisql_sqa_masklm_large.zip. I understand that the model was trained on all datasets and it was pre-trained with MLM objective on the collected wiki tables corpus. Does that mean for SQA and WikiSQL the model was fine-tuned on those tasks or also trained with MLM objective on those datasets?

I would greatly appreciate your feedback on the questions above.
Thanks so much in advance
Volker

We use the official Bert checkpoint but discard the position embedding table. This happens here.

We pre-train with max_seq_length 128 and fine-tune with 512.

This particular model is not described in the paper.
We basically train a chain:

MaskLM pre-training on the pretraining data
Fine-Tune model on SQA
Fine-Tune model on (Supervised) WikiSQL
Fine Tune model on (Weakly-Supervised) WTQ

Trying fine_tuning on Wiki_SQL supervised i got an OOM (out of memory) error

tensorflow.python.framework.errors_impl.ResourceExhaustedError: 2 root error(s) found.
(0) Resource exhausted: OOM when allocating tensor with shape[10,12,512,512] and type float on /job:localhost/replica:
[[{{node bert/encoder/layer_6/attention/self/Softmax}}]]
Hint: If you want to see a list of allocated tensors when OOM happens, add report_tensor_allocations_upon_oom to RunOpti

     [[loss/add_4/_5157]]

Hint: If you want to see a list of allocated tensors when OOM happens, add report_tensor_allocations_upon_oom to RunOpti
(1) Resource exhausted: OOM when allocating tensor with shape[10,12,512,512] and type float on /job:localhost/replica:
[[{{node bert/encoder/layer_6/attention/self/Softmax}}]]
Hint: If you want to see a list of allocated tensors when OOM happens, add report_tensor_allocations_upon_oom to RunOpti
0 successful operations.
0 derived errors ignored.
So I tried to change the maxseq_length  (putin it at 256 or 128) to ease the allocation and I got

(0) Invalid argument: Key: input_ids.  Can't parse serialized Example.
[[{{node ParseSingleExample/ParseExample/ParseExampleV2}}]]
[[IteratorGetNext]]
[[IteratorGetNext/_5085]]
(1) Invalid argument: Key: input_ids.  Can't parse serialized Example.
[[{{node ParseSingleExample/ParseExample/ParseExampleV2}}]]
[[IteratorGetNext]]
0 successful operations.
0 derived errors ignored.
So is 512 the only valid value? Is there on other parameter to change to ease the allocution? Here is the entore command :

command = str('python tapas/tapas/run_task_main.py '+

'--task="WIKISQL_SUPERVISED" '+
'--output_dir="results" '+
'--model_dir="tapas_sqa_inter_masklm_base" '+
'--init_checkpoint="tapas_sqa_inter_masklm_base/model.ckpt" '+
'--bert_config_file="tapas_sqa_inter_masklm_base/bert_config.json" '+
'--mode="train" '+
'--iterations_per_loop=5 '+
'--train_batch_size=10 '+
'--max_seq_length=512')
os.system(command)

You should be able to train models with shorter sequence lengths.
One caveat is that the TF examples have to correspond, that is, when you change the sequence length you need to regnerate the TF examples with the new sequence length.

@muelletm
as I understand your answer I should create data like this :

python3 tapas/run_task_main.py
--task="WIKISQL_SUPERVISED"
--input_dir="WIKISQL\data"
--output_dir="results"
--bert_vocab_file="tapas_sqa_inter_masklm_base/vocab.txt"
--max_seq_length = 256
--mode="create_data"

Am I right?

authors: alxblandin muelletm thomasmueller-google volker42maru

repo: google-research/tapas


text:
Query about the reproducibility of the Motion Capture dataset in "Scalable Gradients..." (Li et al., 2020)

I am trying to reproduce the results of the CMU Motion Capture dataset. I use references from examples/latent_sde_lorenz.py, the paper, and the preprocessed dataset linked by ODE2VAE's repo.
My current runs' results have large discrepancies from the results in the paper so I want to check if there are any training details I'm missing. (I am not very familiar with Bayesian modeling so I try to follow the hyperparameters in the paper as closely as possible.)
Here are the main issues:

The validation and the test MSE for Latent SDE are in the range of 20-30, while Latent SDE's Test MSE in the paper is 4.03 +- 0.20.
The log-likelihood term in the ELBO for Latent SDE is in the magnitude of 10^6 to 10^9 depending on the choice of hyperparameters, while the code for ODE2VAE has the log-likelihood for ODE2VAE in the magnitude of 10^4.

Here are the main training details I think I am most likely to wrongly interpret from the paper:

When calculating the log-likelihood, I am following your example and take the sum over all timesteps and the data dimension, then take the mean over the number of samples. Please let me know if this is correct for the CMU mocap dataset.
In training, the prediction and the log-likelihood should only be calculated for the last 297 predictions (since the first 3 observations are used to encode the context).
The solver used is not mentioned. I have tried euler_heun, milstein, and even reversible_heun.
The initial learning rate for Adam is 0.01 in the paper, but in some runs with the initial lr to be 0.001, they seem to be more stable. I'm curious if you have any comments about this.
The dt for the solver is said to be 1/5 of the minimum time difference between two observations. All observations are regular so we can choose the minimum time to be a particular value a (eg., 1), then dt would be 0.2 * a. I want to know if my interpretation here is correct. The paper didn't mention this value a or the start/end time. It would be nice if you remembered this.

Tagging @lxuechen since you know the most about the exp details. Thank you for your time!

After looking further, I find that my reproduction lacks the KL penalty term (mentioned as the BNN weight penalty in the ODE2VAE paper). The details on how exactly you calculate this term are not mentioned in the paper (e.g whether to calculate the KL penalty at t0 or tN). I would be grateful if you can elaborate further on this.

The initial learning rate for Adam is 0.01 in the paper, but in some runs with the initial lr to be 0.001, they seem to be more stable. I'm curious if you have any comments about this.

0.01 worked for my experiments if you apply reasonable decay.

After looking further, I see that the KL penalty at t0 (KL divergence between p(z) and q(z)) is already included. I am wondering if we also need the KL penalty at tN (KL divergence between p(x|z) and q(x|z))?

You would need KL at time 0, and the KL between the two stochastic processes.
A nice example of how this is computed can be found here for a toy task. logqp_path is what you're looking for. logqp0 is the KL at time 0.

@nghiahhnguyen were you able to reproduce the results for this dataset?
@lxuechen Could you please clarify which KL reweighting coefficient from {1, 0.1, 0.01, 0.001} worked best in your experiments (for LatentODE and LatentSDE)? Also, was this with or without the linear annealing schedule?

I was able to reduce the Test MSE to the range of 8.x - 1x.x after some changes, which is much lower than my first attempt, but still short of the figure 4.03 +- 0.20 in the paper.

Hi @abdulfatir, one point I found the most helpful, which I overlooked in my first attempt, is that the standard deviation of the predicted observations should be learnable (as stated in the paper), but the example used a fixed hyperparameter. When I noticed and changed it, Test MSE dropped significantly to [8.x, 1x.x], as I mentioned above. I'm not sure if you have noticed it already but I guess it's worth mentioning!
Other than that, I don't recall any significant changes I made. In case it helps, I'm listing some significant hyperparameters in my best run:

Use the adjoint method.
Solvers: reversible_heun & adjoint_reversible_heun.
Step size: solve from t0=0.3 to t1=30 with a dt_ratio of 0.2 (I have no specific reasoning for the choice of t0 and t1, just a random thing I tried).
Beta (the coefficient of the KL divergence in the training loss): 1 with a linear schedule of 400 epochs (Looking back at the Val plot, the Val MSE loss starts dropping significantly lower than previous runs at around epoch 500. Val MSE loss starts plateauing around epoch 670. Maybe this is an interesting correlation!)
Gradient clipping: 0.5 (I'm using PyTorch Lightning, so this is done with their built-in implementation).

With the above setting, my log-likelihood is around x1e4 for training and x1e4 for validation and testing. Please let me know if you managed to make further progress!

Thanks a lot @nghiahhnguyen! I did use trainable scales in my experiments but only as a trainable vector (not output from a NN as in the paper).
Thanks for sharing your hyperparameters. I am giving it a go now. Will keep you posted. Are you sure about clipping the grad norm to 0.5? Given the magnitude of gradients for this model, this is quite a small value.
Something I noticed when using scales output from a NN for the observation distribution is that the model becomes very susceptible to crashing with NaNs (possibly due to numerical errors). Did you experience something similar?

I'm glad that I can be of help @abdulfatir.
I also experienced a lot of crashing with NaNs. That's why I started with such a small value of max norm for gradient clipping. I'm using gradient clipping only to stabilize the training process, so I guess you can try larger values and see if the process is still stable. I remembered that trying much larger values does not prevent NaNs-related crashing, but your setup might differ.

Is anyone willing to share their implementation to reproduce the results of the paper? :pleading_face: :pray:

@matteoguarrera Unfortunately, despite trying for several weeks, I wasn't able to reproduce a number anywhere close to what's reported in the paper. Finally, for these datasets, I just copied the number from prior works in our paper: https://arxiv.org/abs/2301.11308

authors: abdulfatir lxuechen matteoguarrera nghiahhnguyen

repo: google-research/torchsde