-
Notifications
You must be signed in to change notification settings - Fork 25
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
updates dependencies #655
updates dependencies #655
Conversation
9217162
to
8fc7647
Compare
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.
Good work!! You managed to modify almost every single file in the repo haha. Though I don't have anything to comment on the code.
-
Can you update the Vus.js section of the Contributing guide with updated information now that we are using Vite?
-
I see you've re-written some logic in the web-client, have you played around in the web client to replace our missing tests? I've quickly trained a titanic model and the line plot doesn't seem to be updated anymore during training for example.
54a21b6
to
16ce26b
Compare
soo, I driffted a bit from the original PR aim, oupsi
|
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.
Lots of good stuff, thanks!!
I got an error specific to silicon chips when trying to build the web-client, I copy-pasted below the stack below. I followed the advice to delete package-lock.json and node-modules and run npm i
and I can now build successfully. However the package-lock.json got modified, so I'm pushing it so you can check it out and decided whether to keep the now one or revert the previous one.
Error stack
> build
> vue-tsc --build && vite build
/Users/.../disco/node_modules/rollup/dist/native.js:59
throw new Error(
^
Error: Cannot find module @rollup/rollup-darwin-arm64. npm has a bug related to optional dependencies (https://github.com/npm/cli/issues/4828). Please try `npm i` again after removing both package-lock.json and node_modules directory.
at requireWithFriendlyError (/Users/.../disco/node_modules/rollup/dist/native.js:59:9)
at Object.<anonymous> (/Users/.../disco/node_modules/rollup/dist/native.js:68:76)
... 3 lines matching cause stack trace ...
at Module._load (node:internal/modules/cjs/loader:1023:12)
at cjsLoader (node:internal/modules/esm/translators:356:17)
at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:305:7)
at ModuleJob.run (node:internal/modules/esm/module_job:218:25)
at async ModuleLoader.import (node:internal/modules/esm/loader:329:24) {
[cause]: Error: Cannot find module '@rollup/rollup-darwin-arm64'
Require stack:
- /Users/.../disco/node_modules/rollup/dist/native.js
at Module._resolveFilename (node:internal/modules/cjs/loader:1144:15)
at Module._load (node:internal/modules/cjs/loader:985:27)
at Module.require (node:internal/modules/cjs/loader:1235:19)
at require (node:internal/modules/helpers:176:18)
at requireWithFriendlyError (/Users/.../disco/node_modules/rollup/dist/native.js:41:10)
at Object.<anonymous> (/Users/.../disco/node_modules/rollup/dist/native.js:68:76)
at Module._compile (node:internal/modules/cjs/loader:1376:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
at Module.load (node:internal/modules/cjs/loader:1207:32)
at Module._load (node:internal/modules/cjs/loader:1023:12) {
code: 'MODULE_NOT_FOUND',
requireStack: [
'/Users/.../disco/node_modules/rollup/dist/native.js'
]
}
}
It looks like updating dependencies created a few breaking changes in the web-client...
-
During training, logs are not displayed in the "Training logs" div, but raw logs get printed in the lower left corner of the screen
-
After training, clicking on Test the model doesn't work and instead of a toaster, the error "Model not trained" gets also printed in the lower left corner of the page
If you have the time and motivation, writing test cases for these errors would be great! 😁
febc1e9
to
9991023
Compare
9991023
to
adb3d3f
Compare
yeah, it's a faster moving space now that dependencies are actual again.
ho, I forgot to actually load a theme in the Toaster! I don't think we really need messages at all (the toaster itself is alreay quite fancy, there are better way to show that's models are training such as via the graphs we have).
ho, it seems that the model is not stored in memory automatically anymore. last commit should fix it.
héhé, and regressions tests added. it's actually super fast to write some with another point: after discussion with Klavdiia, it seems that I forgot to reimplement the calling of |
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.
Looks good to me!
I noticed an error when predicting after training a model and I pushed a fix myself, there's a high chance this bug is my fault...
1edf14c
to
4d56afc
Compare
currently, there is a bunch of security issues, mostly from old dependencies. this PR aims to fix that
vite
instead of the deprecatedvue-cli
strict
compile optionslang="ts"
to all script tagscommonize eslint Linting mismatch #534immutable@5
is still in beta, even ifnpm outdated
marks it as updatableeslint@9
is too recent for plugins to support it yetalso partially fix a race condition when receiving peers in decentralized. it would need a better synchronisation mechanism (an ACK when peer updates are done, like a real RPC) to ensure that slower nodes are indeed ready to communicate.
also rework the cypress tests to remove some uneeded files, and add a test of the training.