Conversation
danieldk
force-pushed
the
card-nix-build
branch
from
ced149d to
7fd3d07
Compare
drbh
left a comment
drbh
left a comment
There was a problem hiding this comment.
lgtm!
question, this seems like this change will make the CARD.md non optional since if I'm reading correctly fill-card will throw if it cant find the template or if theres no repo-id? enforcing a card seems reasonable but just want to understand the change fully
| ] | ||
|
|
||
| [general.hub] | ||
| repo-id = "kernels-test/cutlass-gemm-tvm-ffi" |
There was a problem hiding this comment.
I am guessing the token is configured to write to kernels-test too?
There was a problem hiding this comment.
We don't actually upload these. I just needed to give them an identifier. We could also consider changing these to example/<name>, though that org seems to exist:
| @@ -0,0 +1,48 @@ | |||
| --- | |||
| library_name: kernels | |||
There was a problem hiding this comment.
Not sure? This comes from the template that init uses:
https://github.com/huggingface/kernels/blob/main/kernel-builder/src/init/templates/CARD.md?plain=1
I have no idea where it comes from, maybe @drbh ?
There was a problem hiding this comment.
yea, added because I was using the output from the previous ModelCardData output which included the library name field
that being said its not fully clear if we need it since its really a model loading concept... maybe we can remove (no strong preference)
There was a problem hiding this comment.
I'll leave that to a separate PR. No preference either.
Great catch. I first used a hook where I also checked the presence of |
This ensures that filled cards are in `result` as well (not just `build`) and keeps people that use Nix directly (i.e. me :-)) happy.
danieldk
force-pushed
the
card-nix-build
branch
from
7fd3d07 to
a734ec6
Compare
sayakpaul
left a comment
sayakpaul
left a comment
There was a problem hiding this comment.
Just one comment concerning test as it seems like we're building first then validate card content.
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: DeterminateSystems/nix-installer-action@main | ||
| - uses: DeterminateSystems/nix-installer-action@ef8a148080ab6020fd15196c2084a2eea5ff2d25 |
| - name: Nix checks | ||
| run: nix build .\#checks.x86_64-linux.default | ||
| - name: Test kernel card generation | ||
| run: | | ||
| ( | ||
| cd examples/kernels/silu-and-mul | ||
| nix build -L . | ||
| test -e result/CARD.md | ||
| ) |
There was a problem hiding this comment.
Hmm, do we need to trigger an entire build to check for the card stuff? Seems like an overkill to me 👀
There was a problem hiding this comment.
Or I guess it's simple and doesn't cost much?
There was a problem hiding this comment.
Yeah, it's a noarch kernel, so cheap. We need to do a bundle build to generate the card.
4 participants
This ensures that filled cards are in
resultas well (not justbuild) and keeps people that use Nix directly (i.e. me :-)) happy.