LlamaSharp won't run on legacy NET Framework 4.8

[clovisribeiro]

The call NativeApi.llama_model_default_params() fails when executing LlamaSharp on NET Framework 4.8 due to a limitation/bug in interop code of NET Framework 4.8.

Just for testing, I added a simple code to avoid calling NativeApi.llama_model_default_params() when running on NET Framework 4.8 and everything else executed flawlessly.

I haven't had time to look into this but I am certain there could be a workaround when calling llama_model_default_params() that could avoid the bug.

On a last comment, NET Framework 4.8/C#7.3 does not support asynchronous streams, so I had to use the following little trick to make the IAsyncEnumerable work and use the various async methods provided my LlamaSharp:

`
IAsyncEnumerable async_enum = executor.InferAsync(prompt, inferenceParams);
IAsyncEnumerator enumerator = async_enum.GetAsyncEnumerator();
try
{
while (await enumerator.MoveNextAsync())
{
string text = enumerator.Current;
Console.Write(text);

      if (Console.KeyAvailable)
      {
          inference_cancellled = true;
          break;
      }
  }

}
finally
{
await enumerator.DisposeAsync();
}
`

[martindevans]

I think this is probably the same problem that was reported in #508. If you can work out what exactly it doesn't like I'm sure we can get a workaround in.

My guess is something in LLamaModelParams is upsetting the marshalling, but I'm not sure what.

[clovisribeiro]

The call I mentioned is generating the following exception message: "Method's type signature is not PInvoke compatible."

If you look at https://github.com/dotnet/runtime/issues/10973, it looks like it is the same limitation/bug.

I will see if I can spend sometime on this later to do a more "manual" marshalling so I can have control over the process and avoid the bug.

I think it is worth as there is a lot of legacy (NET 4.8) code out there that could benefit from a great wrapper like LlamaSharp!

[martindevans]

If you're investigating this a good start might be to go through and mark every field in LLamaModelParams with appropriate MarshalAs attributes (docs). This attribute explicitly tells the marshaller how to handle each field.

Obviously these aren't required for modern dotnet, the marshaller is inferring the correct thing for every field. I'm just guessing that inference may be failing on older dotnet framework.

[martindevans]

And by extension also try adding those attributes to LLamaModelMetadataOverride since that's contained within the params struct.

You could also replace all pointers (void*, float*, LLamaModelMetadataOverride*) with a simple IntPtr. That reduces the work the marshaller is doing.

[clovisribeiro]

Ok, for some unknown reason the NET 4.8 marshaller does not like the LlamaProgressCallback delegate inside LLamaModelParams. I tried setting [MarshalAs(UnmanagedType.FunctionPtr)] and also setting a MarshalAs attribute for the delegate but got the same error.

As expected, replacing the LlamaProgressCallback by a generic IntPtr works fine and I could compile and run. The original code is great and I don't really like this solution, specially knowing this is a specific NET 4.8 problem but it does work.

[martindevans]

In that case, would you be interested in submitting a PR that simply wraps that field in an #ifdef to change it into a IntPtr on Framework 4.8 (with a comment explaining why)?

[clovisribeiro]

Glad to do it. The only thing is the targets now are NETStandard;net6.0'net7.0'net8.0, so I would need to add the #if for NETSTANDARD

[martindevans]

In that case I think you'd probably have to do something like #if NETSTANDARD && !NET6_0_OR_GREATER.

[clovisribeiro]

Tested successfully with NET Framework 4.8 and NET 8.0 with:

#if NETSTANDARD2_0 && !NET6_0_OR_GREATER // this code is intended to be used when running LlamaSharp on NET Framework 4.8 (NET Standard 2.0) // as NET Framework 4.8 does not play nice with the LlamaProgressCallback type public IntPtr progress_callback; #else public LlamaProgressCallback progress_callback; #endif

[martindevans]

Looks good! Can you open a PR to get this merged in?