zooniverse / designator

Smart task assignment system
2 stars 1 forks source link

Decimal priorities-Sequential Subject selection #143

Closed yuenmichelle1 closed 2 years ago

yuenmichelle1 commented 2 years ago

Ensure we correctly sort decimal numbers with a Mantissa for prioritized workflows. Before this PR the List.keysort function didn't correctly sort the decimal type values due to the underlying module struct values, see this issue for more details https://github.com/ericmj/decimal/issues/45

As such this PR adds the following to ensure we correctly order all decimal values

camallen commented 2 years ago

@yuenmichelle1 mind having a look over this - as it's your PR I don't think you can formally review it but I think it's worth getting some more eyeballs on it.

camallen commented 2 years ago

Also curious, how often is it that we get priority being decimals? (ISTROX was the first time I've encountered, but I was under the assumption priority would always be integers?)

Always - under the hood the API database stores them as postgres numeric which maps to a Decimal via Ecto https://github.com/elixir-ecto/ecto. The original idea was to allow higher precision grouping via the decimal form.

Here is where we query the API set_member_status table https://github.com/zooniverse/designator/blob/90986ea5616844d240be3c0f8d0c38379bd02f04/web/models/workflow.ex#L29 and thus align to the underlying postgres numeric / decmial result type.

Note: here is where we setup our test schema to match the API one - https://github.com/zooniverse/designator/blob/1e6d101de55e60ec3d391885195ae911484c0ee7/priv/repo/migrations/20160819141144_default_tables_for_testing.exs#L28-L34

yuenmichelle1 commented 2 years ago

I tested this with ISTROX locally (by hooking up readonly db to local running designator) and the output ids are matching what cellect is also outputting. I also tested a project where set_member_subjects has integer priority and it works as expected as well 👍 🎉 🥂