linfa icon indicating copy to clipboard operation
linfa copied to clipboard

linfa-preprocessing::countgrams: RefCell is replaced with OnceLock

Open kakserpom opened this issue 7 months ago • 7 comments

Simple fix to allow for concurrent access.

kakserpom avatar May 03 '25 16:05 kakserpom

OnceLock is not serde. What about cloning to use multi-threading?

relf avatar May 19 '25 09:05 relf

@relf I don't quite understand what you mean by that. OnceLock is a better fit here than RefCell because it's thread-safe.

kakserpom avatar May 21 '25 17:05 kakserpom

With OnceLock we cannot serialize/deserialize CountVectorizerValidParams which is a no go (cf. CI tests errors).

I meant you can just clone CountVectorizerParams and use an independent copy in each thread (if it was your initial problem with the multi-threading).

relf avatar May 21 '25 19:05 relf

@relf Well, I guess we could solve the issue by implementing serialize and deserialize manually, right?

kakserpom avatar May 22 '25 09:05 kakserpom

Maybe... But there is no problem in the first place if you just clone CountVectorizerParams so no need of a special serde code complication.

relf avatar May 26 '25 13:05 relf

@relf please review the updated version.

kakserpom avatar Jun 10 '25 20:06 kakserpom

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 36.17%. Comparing base (5272ad1) to head (0a23fc7). Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
+ Coverage   35.90%   36.17%   +0.26%     
==========================================
  Files          97      101       +4     
  Lines        6409     6563     +154     
==========================================
+ Hits         2301     2374      +73     
- Misses       4108     4189      +81     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Jun 10 '25 22:06 codecov[bot]