cadia-lvl / samromur

https://samromur.is/
MIT License
2 stars 2 forks source link

removed ordering from H17-L2 sentence query #147

Closed 17luke closed 2 months ago

17luke commented 5 months ago

@thdg Removing random ordering seems to speed up the query at least 5x when testing on my connection.

I'm guessing the random ordering had something to do with copyright for the main collection. That said, the l2 sentences already seemed to be randomised in the table.

judyfong commented 5 months ago

Only one way to know, ask the authors. What was the date? Yall know who was working at lvl and samromur at the time. But yall can also reference the samromur papers written by carlos and staffan for lrec2022. That should cover everything and theyre short.

Btw thorsteinn i never had time to ask jon if you could go with at the 2022 lrec. Im sorry. I know i said would ask and youd expressed interest. Its too bad cause i think you wouldve like the food and culture there like ur dad did.

On Friday, June 7, 2024, 17luke @.***> wrote:

@thdg https://github.com/thdg Removing random ordering seems to speed up the query at least 5x when testing on my connection.

Only question is whether the random ordering had something to do with copyright. That said, the l2 sentences already seemed to be randomised in the table.

You can view, comment on, or merge this pull request online at:

https://github.com/cadia-lvl/samromur/pull/147 Commit Summary

File Changes

(1 file https://github.com/cadia-lvl/samromur/pull/147/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/cadia-lvl/samromur/pull/147, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUMNYBJN5GGCIV5NS4UKTTZGHQMVAVCNFSM6AAAAABI67ZFISVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2DAOBTG44DENY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

17luke commented 5 months ago

Note this change doesn't affect any other samromur projects/collections. It's an edit to a function which is only called on the new L2 collection sentences that were added by Thorsteinn. This is to help solve the issue of L2 sentences taking a very long time to appear for users on samromur.is/l2

thdg commented 5 months ago

No copyright problems. Sentences are already shuffled. Only problem I see is if more than one users are reading at the same time they will get the same sentence list. Even if one has started (and finished like 10) the next one will get the same 40 first one has left and then 10 more. We can sidestep this by doing the shuffling ourselves in code, fetch maybe 10 times what we need and then shuffle those. There will be some repeats but not enough to cause a problem I think.

17luke commented 2 months ago

Covered in commit https://github.com/cadia-lvl/samromur/commit/e93e0bc1a08dcc1f42c6bade6705475b709c33b9

Closing PR