Improve SourcePawn heuristics #5479

rtldg · 2021-07-24T04:30:24Z

Some of the SourcePawn include files (.inc) in https://github.com/shavitush/bhoptimer/tree/master/addons/sourcemod/scripting/include are being picked up as C++ for the repository's language percentage.

addons/sourcemod/scripting/include/closestpos.inc (via linguist)
addons/sourcemod/scripting/include/convar_class.inc (on language page)
addons/sourcemod/scripting/include/dhooks.inc (on language page)
addons/sourcemod/scripting/include/shavit.inc (via linguist)

Description

This adds some more patterns to heuristics.yml to match for the following SourcePawn syntax:

methodmap ExtendedType < BaseType
~~stock returntype FunctionName()~~
~~native returntype FunctionName()~~
~~forward returntype FunctionName()~~

Checklist:

I am fixing a misclassified language
- I have included a new sample for the misclassified language:
  - Sample source(s):
    - https://github.com/shavitush/bhopstats/blob/9ad328ccdf7ac44f61c190d7fcebe7420d3b9ece/scripting/include/bhopstats.inc
    - https://github.com/shavitush/bhoptimer/blob/da9144f1b5fbc2a9d7d2153f3a584a75c7218a90/addons/sourcemod/scripting/include/shavit.inc
  - Sample license(s):
    - GPLv3
- I have included a change to the heuristics to distinguish my language from others using the same extension.

rtldg · 2021-07-24T10:06:38Z

Looks like it caused the Pawn tests to fail as it detected it as SourcePawn. Can I have some guidance on what I should do about this?

lildude · 2021-07-24T10:09:30Z

You'll need to adjust your heuristic to ensure it only captures SourcePawn-specific syntax. If that's not possible, then your only option is to close this PR and implement an override in your repo.

rtldg · 2021-07-24T11:21:36Z

I changed the heuristic that was catching Pawn files and the tests pass on my local computer now.
The file shavit.inc is still recognized as C++ though. Would it be fine to add that as a SourcePawn sample? I didn't know that adding a sample affects the classifier at the start but it seems just dropping that file into the samples would've caused all the files to be correctly recognized as SourcePawn and I wouldn't have needed to add heuristics.

lildude · 2021-07-25T07:10:45Z

Would it be fine to add that as a SourcePawn sample? I didn't know that adding a sample affects the classifier at the start but it seems just dropping that file into the samples would've caused all the files to be correctly recognized as SourcePawn and I wouldn't have needed to add heuristics.

You could, and adding it would have the effect of helping correctly identify all the files in your repo as it increases the chances the tokens match. I think a question needs to asked first: what specifically about the file (on its own) makes it SourcePawn as opposed to Pawn or C++? Identifying that could help improve the heuristic even more so others get the benefit too.

Also, keep in mind we don't aim for 100% accuracy with Linguist, so an override may need to be used in particularly tricky or vague scenarios.

rtldg · 2021-07-25T08:32:42Z

I think a question needs to asked first: what specifically about the file (on its own) makes it SourcePawn as opposed to Pawn or C++?

Mainly the SourceMod SharedPlugin and SetNTVOptional stuff. Back in 2014 or so SourcePawn's syntax was expanded though to be more C/C++ -like (while still maintaining compatibility with the old syntax). Things like methodmap, view_as<float>(somerandomint), and enum struct are now good indicators that a file is SourcePawn instead of Pawn if the SharedPlugin or SetNTVOptional stuff isn't in a file.

As for differentiating between SourcePawn and C++, the methodmap, view_as<float>(somerandomint), and the function modifiers forward, stock, and native would be good ways. C++ also has enum struct so that wouldn't work. The forward, stock, and native things already caused problems with making Pawn be recognized as SourcePawn so I'm not sure how that can be used well.

So for why that file is recognized as C++: Making a skeleton .inc file with only the content asdf inside causes the file to be recognized as C++ for me so I think the C++ classifier is just too strong and the heuristics aren't able to override it since modern SourcePawn already looks similar to C++.

lildude · 2021-07-25T10:29:06Z

Making a skeleton .inc file with only the content asdf inside causes the file to be recognized as C++ for me so I think the C++ classifier is just too strong and the heuristics aren't able to override it since modern SourcePawn already looks similar to C++.

There are no heuristics so the classifier is the only thing doing the guessing and it is notoriously bad at small samples, especially if there are no language-specific tokens in it. This happens a lit with C-family header files.

asdf is also a really terrible way to try test a language analyser based on content on its own without any other context, you'd never guess the language… its not even English 😁

lildude · 2021-07-25T10:38:13Z

Oh yes, C++ came out as the result for your test because it's alphabetically before Pawn and SourcePawn and not because of the content or anything the classifier knows about as none of the samples for these languages have asdf as a token 😁

lildude · 2021-07-26T04:45:03Z

There are no heuristics

Whoops, ignore that. I was mixing up my discussions. However, it's still kinda true as the heuristics wouldn't match asdf so wouldn't help in this scenario.

lildude · 2021-07-26T08:31:13Z

Mainly the SourceMod SharedPlugin and SetNTVOptional stuff.

🤔 I've taken a closer look at your shavit.inc file and it has both of those as does the heuristic so the current heuristic should be correctly detecting the language of that file. I'm not sure why it's not doing so though.

lildude · 2021-07-26T08:38:44Z

Worked it out... those lines happen quite late into the file and the heuristics only analyse the first 50kb of the file:

https://github.com/github/linguist/blob/c492ac0d5bc697033c4261e8bfb597949ea882e2/lib/linguist/heuristics.rb#L6-L24

I'm not keen on increasing this as it's set low to keep things performant so I think it makes sense to add a sample that contains these tokens for the few cases like yours where we fall through to the classifier.

rtldg · 2021-07-27T09:42:02Z

Worked it out... those lines happen quite late into the file and the heuristics only analyse the first 50kb of the file:

That definitely explains what was happening then.

I'm not keen on increasing this as it's set low to keep things performant so I think it makes sense to add a sample that contains these tokens for the few cases like yours where we fall through to the classifier.

Understandable. A couple of good samples that I think would be good to use:

shavit.inc - enum struct, SharedPlugin, SetNTVOptional, char[] buffer, default parameter values
- Actually, this is still too big for the heuristics to reach the end of. Although I could just move the SharedPlugin and SetNTVOptional stuff to the top and/or strip the non-license comments.
dhooks.inc - typeset, methodmap, SetNTVOptional
bhopstats.inc - methodmap, methodmap properties, SharedPlugin, SetNTVOptional

And I guess test_heuristics.rb:test_inc_by_heuristics should have SourcePawn moved out of the nil key and swapped to *.inc

Also, I found is that mfile.inc isn't SourcePawn. It's actually Pawn and seems to be for open.mp, a GTA: San Andreas mod based on ~~API used~~ the authors (Minokon & Y-Less) and commit it was added in

I'll wait to see if you have any thoughts before adding more commits for any of it.

lildude · 2021-07-27T09:49:42Z

Understandable. A couple of good samples that I think would be good to use:

* [`shavit.inc`](https://github.com/shavitush/bhoptimer/blob/da9144f1b5fbc2a9d7d2153f3a584a75c7218a90/addons/sourcemod/scripting/include/shavit.inc) - `enum struct`, `SharedPlugin`, `SetNTVOptional`, `char[] buffer`, default parameter values

* [`dhooks.inc`](https://github.com/peace-maker/DHooks2/blob/008a2fd0220c7999e3c6337faf0e69c3b04f7bda/sourcemod_files/scripting/include/dhooks.inc) - `typeset`, `methodmap`, `SetNTVOptional`

* [`bhopstats.inc`](https://github.com/shavitush/bhopstats/blob/9ad328ccdf7ac44f61c190d7fcebe7420d3b9ece/scripting/include/bhopstats.inc) - `methodmap`, methodmap properties, `SharedPlugin`, `SetNTVOptional`

I think adding all three will be good for the classifier.

And I guess test_heuristics.rb:test_inc_by_heuristics should have SourcePawn moved out of the nil key and swapped to *.inc

Yup. 👍

Also, I found is that mfile.inc isn't SourcePawn. It's actually Pawn and seems to be for open.mp, a GTA: San Andreas mod based on ~~API used~~ the author (Y-Less) and commit it was added

If you're 💯 on this, then lets move it to Pawn.

rtldg · 2021-07-27T12:26:50Z

Number of errors (39) is above the acceptable threshold (38)

Looks like the classifier cross-validation error count increased from that I think from these:

Pascal/GraphicConfiguration.inc BAD (SourcePawn)
- dhooks.inc causes this for some reason
Pawn/fixed.inc BAD (SourcePawn)
Pawn/mfile.inc BAD (SourcePawn)

Is this a problem or a just a situation where the ACCEPTABLE_ERRORS gets bumped? It should only be decreased. 🤔

https://github.com/github/linguist/blob/a3a3b8b330d296589f103d322303826a4c2bcfda/script/cross-validation#L3-L5

lildude · 2021-07-27T13:33:14Z

Is this a problem or a just a situation where the ACCEPTABLE_ERRORS gets bumped?

Yes, this is a problem as we're trying to improve the classifier. The increase in errors is an indication that these changes have caused something to now be incorrectly classified.

master has the following known misclassifications around Pawn and SourcePawn:

Pawn/fixed.inc BAD (SourcePawn)
SourcePawn/mfile.inc BAD (C++)

This branch has these:

Pascal/GraphicConfiguration.inc BAD (SourcePawn)  <--- This is new and definitely unwanted as it was correct before
Pawn/fixed.inc BAD (SourcePawn)
Pawn/mfile.inc BAD (SourcePawn)

I suspect this might be happening because of the amount of duplication of syntax in the large files you've added which appears to be swaying the classifier towards SourcePawn.

dhooks.inc causes this for some reason

If this is the case, then lets remove it. This should take us back down to the two known incorrect classifications which will improve if and when more diverse samples are added for Pawn in future.

Unrelated, it looks like the boo grammar has gone MIA. I'll see if I can fix that in a separate PR.

rtldg · 2021-07-27T13:54:29Z

dhooks.inc causes this for some reason

If this is the case, then lets remove it. This should take us back down to the two known incorrect classifications which will improve if and when more diverse samples are added for Pawn in future.

Alright, done for that.

Unrelated, it looks like the boo grammar has gone MIA. I'll see if I can fix that in a separate PR.

That got me too when I bootstrapped a fresh repo and had to keep alt-tabbing back to my terminal wondering why it was taking so long.

lildude

All good and looks good to me now. Thanks for being so patient. 🙇

…5479) * Extend SourthePawn regex * Move mfile.inc to Pawn samples * Add more `.inc` SourcePawn samples

add a few sourcepawn heuristics

dafe2e1

rtldg requested a review from a team as a code owner July 24, 2021 04:30

adjust sourcepawn .inc heuristic that was catching pawn files

0ce105c

rtldg added 2 commits July 27, 2021 10:12

move mfile.inc to Pawn samples

d1119d8

add more .inc samples for SourcePawn

bbfcd04

remove dhooks.inc since it adds more classifier cross-validation errors

5cd8759

Merge branch 'master' into sourcepawn-heuristics2

0a76c77

lildude approved these changes Jul 27, 2021

View reviewed changes

lildude merged commit ca8b002 into github-linguist:master Jul 27, 2021

lildude changed the title ~~Add a few SourcePawn heuristics~~ Improve SourcePawn heuristics Jul 27, 2021

kalkin added a commit to kalkin/file-expert that referenced this pull request Sep 19, 2021

improvement: Improve SourcePawn heuristics (github-linguist/linguist#…

2ab5b9b

…5479) * Extend SourthePawn regex * Move mfile.inc to Pawn samples * Add more `.inc` SourcePawn samples

github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve SourcePawn heuristics #5479

Improve SourcePawn heuristics #5479

rtldg commented Jul 24, 2021 •

edited

Loading

rtldg commented Jul 24, 2021

lildude commented Jul 24, 2021

rtldg commented Jul 24, 2021

lildude commented Jul 25, 2021 •

edited

Loading

rtldg commented Jul 25, 2021

lildude commented Jul 25, 2021

lildude commented Jul 25, 2021

lildude commented Jul 26, 2021

lildude commented Jul 26, 2021 •

edited

Loading

lildude commented Jul 26, 2021

rtldg commented Jul 27, 2021 •

edited

Loading

lildude commented Jul 27, 2021

rtldg commented Jul 27, 2021

lildude commented Jul 27, 2021 •

edited

Loading

rtldg commented Jul 27, 2021

lildude left a comment

Improve SourcePawn heuristics #5479

Improve SourcePawn heuristics #5479

Conversation

rtldg commented Jul 24, 2021 • edited Loading

Description

Checklist:

rtldg commented Jul 24, 2021

lildude commented Jul 24, 2021

rtldg commented Jul 24, 2021

lildude commented Jul 25, 2021 • edited Loading

rtldg commented Jul 25, 2021

lildude commented Jul 25, 2021

lildude commented Jul 25, 2021

lildude commented Jul 26, 2021

lildude commented Jul 26, 2021 • edited Loading

lildude commented Jul 26, 2021

rtldg commented Jul 27, 2021 • edited Loading

lildude commented Jul 27, 2021

rtldg commented Jul 27, 2021

lildude commented Jul 27, 2021 • edited Loading

rtldg commented Jul 27, 2021

lildude left a comment

Choose a reason for hiding this comment

rtldg commented Jul 24, 2021 •

edited

Loading

lildude commented Jul 25, 2021 •

edited

Loading

lildude commented Jul 26, 2021 •

edited

Loading

rtldg commented Jul 27, 2021 •

edited

Loading

lildude commented Jul 27, 2021 •

edited

Loading