RWKV / rwkv.cpp

INT4/INT5/INT8 and FP16 inference on CPU for RWKV language model
MIT License
1.37k stars 90 forks source link

Various improvements & upgrade ggml #75

Closed saharNooby closed 1 year ago

saharNooby commented 1 year ago

Notable changes:

saharNooby commented 1 year ago

@LoganDark Maybe want to take a look before I merge? :)

LoganDark commented 1 year ago

looks like you ran an auto-formatter that messed up some of the assert statements. was that intended?

saharNooby commented 1 year ago

@LoganDark It's intended, to keep the line not too long. Did asserts break because of this?.. Code compiled and tests passed, looks like they still work

LoganDark commented 1 year ago

@LoganDark It's intended, to keep the line not too long.

some of the asserts get long but should still be in only 1 line. An example is if they are in the middle of a list of other one-line asserts and splitting it into a block would be very not pretty. I think the max line length should be set higher or completely disabled.

Also you should probably add a workflow to check code formatting so that we do not have to wait for your next PR for the code to be reformatted

saharNooby commented 1 year ago

if they are in the middle of a list of other one-line asserts and splitting it into a block would be very not pretty

I agree that there is value in symmetry of having lined-up single line statements. For me tho, it's okay to trade it off for the possibility of having all the code visible on the screen without the need to scroll long lines.

add a workflow to check code formatting

That's a good idea, I've added it to my backlog.

LoganDark commented 1 year ago

For me tho, it's okay to trade it off for the possibility of having all the code visible on the screen without the need to scroll long lines.

I don't think the lines are so long that becomes an issue, but whatever I suppose

LoganDark commented 1 year ago

I forgot about the new Q8_1 ;D luckily #74 includes it