-
Notifications
You must be signed in to change notification settings - Fork 319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] NUSMods Optimizer Prototyping #3296
base: master
Are you sure you want to change the base?
Conversation
* Optimizer button next to Exam Calendar * Placeholder component with a hr and text * Redux actions and reducers for maintaining optimizer state across tab changes Redux state is necessary since we want optimizer settings to be maintained even if user navigates to check other modules
@frizensami is attempting to deploy a commit to the NUSMods Team on Vercel. A member of the Team first needs to authorize it. |
This pull request is being automatically deployed with Vercel (learn more). nusmods-export – ./export🔍 Inspect: https://vercel.com/nusmodifications/nusmods-export/F6wr24apyRy6p4hGMibo8L7cBmFq nusmods-website – ./website🔍 Inspect: https://vercel.com/nusmodifications/nusmods-website/GcyXY4La9EfW3SnYE5Xdw1zfScDP |
Codecov Report
@@ Coverage Diff @@
## master #3296 +/- ##
==========================================
+ Coverage 53.16% 54.46% +1.29%
==========================================
Files 270 280 +10
Lines 5720 6274 +554
Branches 1323 1404 +81
==========================================
+ Hits 3041 3417 +376
- Misses 2679 2857 +178
Continue to review full report at Codecov.
|
* z3w.wasm is precompiled from https://github.com/cpitclaudel/z3.wasm/ * z3w.js is the Emscripten generated wrapper from z3w.wasm
website/src/types/optimizer.ts
Outdated
/** | ||
* Defs for communicating between Optimizer <-> WebWorker <-> WASM wrapper | ||
* */ | ||
export enum Z3MessageKind { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there something stopping us from using numbers instead? Iirc, when we pass things to wasm, it is more efficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Z3 message is maybe a misnomer, it's a message for the Z3 Worker. I'll rename it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little easier to debug with string enums, so if it's not a performance concern (very few messages are sent generally), I think the string enum might be ok.
This is a library we control, provides an API for us to create SMT-LIB 2.0 code to pass to Z3. Fork of https://github.com/stanford-oval/node-smtlib + additional APIs for our use case
Context
This WIP PR accompanies the discussions on timetable optimization in #3294.
This PR is for rapid prototyping based on the discussions in the issue + auto deployment to Vercel so that changes can be viewed easily.
Implementation
Note: won't try to fulfil
npm run ci
warnings/errors mostly for now, will implement functionality firstDiscussion points
Some discussion points in no particular order:
Implementation Plan
After further discussion I can update this PR with an implementation plan. Minimally, this needs:
smtlib-ext
library so we can stop the github referenceOf the above, the Constraints UI will need the most work. The latter two already have been somewhat implemented in the standalone NUS Timetable Optimizer.
Translation utils:
Known bugs / limitations
NumericWeeks: number[]
) and notWeekRange
(strange week ranges)Things to test
Current changes
These are just to indicate the idea, please no punterino on the UI :D
Other Information
This is more or less the first time I'm using React, and I'm not much of a front-end designer (clearly, haha), so I'll try to keep up! 🙂