Initial changes for Recurrent state rollback for nemotron for cuda - #26623
Conversation
|
Hi @lnigam, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
There was a problem hiding this comment.
The main problem atm is that I missed the fact that when the new operator is not supported, we don't just need to update the compute graph - we also have to communicate back to the app that the requested n_rs_seq was not satisfied. This gets messy as it requires some extra logic outside of libllama and generally it is something that I would like to avoid.
My suggestion is to drop the llm_fused_op_probe and cparams.fused_ssm_scan related stuff and directly implement the new op across all backends. This is what we actually did for the ggml_gated_delta_net op - a very similar case to this one:
We can do that because the change in the backends is relatively minimal and an agent should be able to do it correctly single-shot. Just make sure the provided tests in test-backend-ops.cpp are good enough in order to catch in the CI if some of the backends failed it (I think the current ones are good).
cc @gaugarg-nv for vis, as you successfully did this for the ggml_gated_delta_net op before.
If this turns to be difficult to do for all the backends, then a simpler option is to just do it for the CPU and CUDA backend (i.e. you already did that) and then mark all backends that they do not support the op. This will cause the computation to fallback to CPU on these devices until it gets implemented. But that should be fine and more importantly, the rollback logic would still work - it would just be a bit slower, since it is going to run on the CPU.
…kend-ops for cpu and cuda
Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
…ed or not and also handled memory->n_rs_seq >1 case incase of op is not supported
3483a70 to
a078c8a
Compare
* Initial changes for Recurrent state rollback for nemotron for cpu and cuda * Removing CPU RS rollback. Will enable it in subsequent PRs * addition of test case * Removing assert and calling runtime API to check if op is supported * removing extra API and updating the call sites for K * replace static cuda detection to runtime fused_op api * address review comments and fallback when SSM rollback not supprted * Adding changes for supporting RS-rollback in CPU. Also added test-backend-ops for cpu and cuda * removing memory manipulation as rs rollback is now supported in CPU * removing the static probe which is not needed now * correcting the format * address review comments * enabling test for all the backends, unsupported backends will fallback to CPU * Apply suggestions from code review Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> * choose different graph based on the result of fused_ssm_op is supported or not and also handled memory->n_rs_seq >1 case incase of op is not supported * Support K > 1 in ssm_scan for all backends * Fix CI Issues --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> Co-authored-by: Gaurav Garg <gaugarg@nvidia.com> (cherry picked from commit 1692f9e)
* Initial changes for Recurrent state rollback for nemotron for cpu and cuda * Removing CPU RS rollback. Will enable it in subsequent PRs * addition of test case * Removing assert and calling runtime API to check if op is supported * removing extra API and updating the call sites for K * replace static cuda detection to runtime fused_op api * address review comments and fallback when SSM rollback not supprted * Adding changes for supporting RS-rollback in CPU. Also added test-backend-ops for cpu and cuda * removing memory manipulation as rs rollback is now supported in CPU * removing the static probe which is not needed now * correcting the format * address review comments * enabling test for all the backends, unsupported backends will fallback to CPU * Apply suggestions from code review Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> * choose different graph based on the result of fused_ssm_op is supported or not and also handled memory->n_rs_seq >1 case incase of op is not supported * Support K > 1 in ssm_scan for all backends * Fix CI Issues --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> Co-authored-by: Gaurav Garg <gaugarg@nvidia.com>
* Initial changes for Recurrent state rollback for nemotron for cpu and cuda * Removing CPU RS rollback. Will enable it in subsequent PRs * addition of test case * Removing assert and calling runtime API to check if op is supported * removing extra API and updating the call sites for K * replace static cuda detection to runtime fused_op api * address review comments and fallback when SSM rollback not supprted * Adding changes for supporting RS-rollback in CPU. Also added test-backend-ops for cpu and cuda * removing memory manipulation as rs rollback is now supported in CPU * removing the static probe which is not needed now * correcting the format * address review comments * enabling test for all the backends, unsupported backends will fallback to CPU * Apply suggestions from code review Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> * choose different graph based on the result of fused_ssm_op is supported or not and also handled memory->n_rs_seq >1 case incase of op is not supported * Support K > 1 in ssm_scan for all backends * Fix CI Issues --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> Co-authored-by: Gaurav Garg <gaugarg@nvidia.com>
Initial changes for Recurrent state rollback for nemotron for cpu and cuda
Overview
This PR adds support for Recurrent state rollback for nemotron model. It creates separate copies of states for every draft token and then rollback to the last accepted token's state. Also added test case to verify the ssm support
Additional information
Requirements