search

numbas / Numbas

A completely browser-based e-assessment/e-learning system, with an emphasis on mathematics
http://www.numbas.org.uk
Apache License 2.0
200 stars 118 forks source link

A 'weighted_random' function #839

Closed christianp closed 3 years ago

christianp commented 3 years ago

There should be a function weighted_random which works like random, but takes a list of probabilities for each item.

I don't know if it would be better for the signature to be (collection, probabilities) or (list of [item, probability]).

Samir70 commented 3 years ago

Hi I could give this a go at the weekend. I've done a weighted probability question on leetcode: https://leetcode.com/problems/random-pick-with-weight/ so could probably manage this.

I've submitted work to other github repos, but might need a bit of help get started on a repo this size.

christianp commented 3 years ago

@Samir70 thanks for offering to help! Please have a look at our page on contributing code to Numbas. You should add a function weighted_random in runtime/scripts/math.js, and then define a JME function which calls it in runtime/scripts/jme-builtins.js.

Let me know if you need any more help. I'm mostly on annual leave at the moment, only working one day a week, so I might be slow to respond.

kalpitf1 commented 3 years ago

@christianp I have made some changes locally to runtime/scripts/math.js and runtime/scripts/jme-builtins.js with the function signature of (list of [item, probability]). I have only cloned the Numbas runtime. Which files in Numbas/tests need to be updated when writing unit tests which I am running using the instructions "Start a local web server with python -m http.server and go to http://localhost:8000/tests"?

Samir70 commented 3 years ago

@kalpitf1 Could I have a look at your code, please? I looked at your fork and the changes don't seem to be there. And I tried over the weekend to figure out how everything in the project is connected and what changes I need to make to maths.js and jme-builtins.js. The actual coding for the weighted random function wouldn't be a problem if I simply knew where to put it. :)

kalpitf1 commented 3 years ago

@Samir70 I have not pushed to my forked repo as I am yet to write the corresponding unit tests for weighted_random. I can open a PR as soon as I do so. Just looking to make my first open source contribution :)

Samir70 commented 3 years ago

@kalpitf1 I suppose I can wait. I had my chance over the weekend. I just want to get to know the codebase a bit better so I can get involved.

Samir70 commented 3 years ago

Thanks for posting @kalpitf1 In the jme builtins, you put the reference between prod and deal. The order of the functions isn't necessarily following a pattern. But I would have thought putting this function next to the random function already there would make more sense.

I guess it doesn't matter.

christianp commented 3 years ago

Thanks for getting this started, @kalpitf1! If you'd like, could you add your name to CONTRIBUTORS.md?

kalpitf1 commented 3 years ago

Thank you for your feedback, @christianp! I will keep those in mind for future contributions. I have added my name to CONTRIBUTORS.md in #845.