Skip to content

Integrate Nscale-cloud into HuggingFace inference #1260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Apr 24, 2025

Conversation

nbarr07
Copy link
Contributor

@nbarr07 nbarr07 commented Mar 7, 2025

Integrate Nscale provider for HuggingFace Inference

This PR adds support for Nscale inference to the HuggingFace inference API.

Note that our inference service is not publicly live yet but will be soon - this draft PR is for review and preparation.

  • Implemented standard provider integration for nscale
  • Added support for text-generation, conversational, and text-to-image tasks
  • Included tests following the established patterns

The tests were all passing when I tried. Any feedback are welcomed!

@nbarr07 nbarr07 marked this pull request as draft March 7, 2025 15:56
@nbarr07 nbarr07 marked this pull request as ready for review March 28, 2025 09:08
@Wauplin
Copy link
Contributor

Wauplin commented Mar 28, 2025

Hey @nbarr07 , thanks for your PR :) Even though we are looking forward to integrate new providers, we'll have to hold on for a bit on this one. Integrating a new provider takes time and effort on our side to make things right so for now we've decided to focus on the existing ones. Will let you know when we are ready to move forward! 🤗

@julien-c julien-c added the inference-providers integration of a new or existing Inference Provider label Apr 2, 2025
@julien-c
Copy link
Member

julien-c commented Apr 2, 2025

Hi @nbarr07 we are currently finishing a refactoring of Inference Providers integration code in #1315, this should be merged soon, but we will need to rewrite part of your implementation (should be even simpler to integrate), will ping again after it's been merged.

@nbarr07
Copy link
Contributor Author

nbarr07 commented Apr 2, 2025

Hi @julien-c , thanks for the heads-up about the refactoring, if there's anything we can do on our side let me know!

@hanouticelina
Copy link
Contributor

Hi @nbarr07,
We've merged a refactoring for Inference Provider integration into main, which should make adding new providers much easier.
Could you merge main into your branch and update your PR accordingly? it should be relatively straightforward with the new structure:
1 - You have to update the PROVIDERS mapping here: inference/src/lib/getProviderHelper.ts#L49 and add nscale-cloud (let's ensure we respect the alphabetical order) :

import * as NScaleCloud from "../providers/nscale-cloud";
...
export const PROVIDERS: Record<InferenceProvider, Partial<Record<InferenceTask, TaskProviderHelper>>> = {
	...
	"nscale-cloud": {
		"text-to-image": new NScaleCloud.NScaleCloudTextToImageTask(),
		"conversational": new NScaleCloud.NScaleCloudConversationalTask(),
		"text-generation": new NScaleCloud.NScaleCloudTextGenerationTask(),
	},
	},
        ...

2 - Update packages/inference/src/providers/nscale-cloud.ts to implement the classes:

import { BaseConversationalTask, BaseTextGenerationTask } from "./providerHelper";

const NSCALE_API_BASE_URL = "https://inference.api.nscale.com";

export class NScaleCloudConversationalTask extends BaseConversationalTask {
	constructor() {
		super("nscale-cloud", NSCALE_API_BASE_URL);
	}
}

export class NScaleCloudTextGenerationTask extends BaseTextGenerationTask {
	constructor() {
		super("nscale-cloud", NSCALE_API_BASE_URL);
	}

	override makeRoute(): string {
		return "v1/chat/completions";
	}
}

export class NScaleCloudTextToImageTask extends TaskProviderHelper implements TextToImageTaskHelper {
	constructor() {
		super("nscale-cloud", NSCALE_API_BASE_URL);
	}

	makeRoute(): string {
		return "v1/images/generations";
	}

	preparePayload(params: BodyParams): Record<string, unknown> {
		// refer to Nebius implementation of preparePayload
	}
         
         getResponse(
		response: unknown,
		url?: string,
		headers?: HeadersInit,
		outputType?: "url" | "blob"
	): Promise<string | Blob>{
		// Refer to Nebius implementation of getResponse
	}
}

You can check Nebius implementation of text-to-image, it looks like it's the same payload and response format as NScale-cloud.
let us know if you need any help! you can find more details in the documentation : https://huggingface.co/docs/inference-providers/register-as-a-provider#2-js-client-integration.

@nbarr07
Copy link
Contributor Author

nbarr07 commented Apr 14, 2025

Hey @hanouticelina,
thanks for the heads up, I've merged the latest changes into my branch and updated the PR. One question I have is does the provider name in inference need to march the organisation username on Huggingface or can they be different?
Let me know if there's any adjustments needed.

@julien-c
Copy link
Member

it's better if they're the same, do you want us to check if we can move your org to nscale namespace?

@nbarr07
Copy link
Contributor Author

nbarr07 commented Apr 14, 2025

yes that would be great, thanks!

Copy link
Contributor

@SBrandeis SBrandeis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there - thank you for your conribution!

I have two minor comments - otherwise the PR looks good !

Ping @hanouticelina for a quick validation before merging

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Contributor

@hanouticelina hanouticelina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thank you @nbarr07!

@nbarr07
Copy link
Contributor Author

nbarr07 commented Apr 15, 2025

@hanouticelina @SBrandeis thank you for the review and changes!
I've just made a last change in the readme (the namespace had to be modified). Lmk when we can merge it, thanks!

@nbarr07
Copy link
Contributor Author

nbarr07 commented Apr 22, 2025

Hey @hanouticelina @SBrandeis , just to get an idea, when could we expect to merge the PR?

@hanouticelina
Copy link
Contributor

let's merge this, thank you @nbarr07 again for the contribution!

@hanouticelina hanouticelina merged commit 6ed9d44 into huggingface:main Apr 24, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inference-providers integration of a new or existing Inference Provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants