Closed mubaidr closed 3 years ago
I'd like to start working on this, I have some experience with Ts, can you tell me which module is up for grabs?
@yashshah1 You are welcome to contribute, please pick any module or portion for conversion and mention it here, so I can mark it in-progress (to avoid any duplication effort).
I'd like to work on this too, any recommendations on where to start?
@nabeelvalley here is the short guide, super simple to start. Please pick any module or portion for conversion and mention it here, so I can mark it in-progress (to avoid any duplication effort).
Here you can find a guide on how to contribute.
@mubaidr I'm working from the utilities
up, to avoid having untyped dependencies
The tests seem to be failing when I convert the file to JS because it looks like Jest is running against the TS files instead of the compiled JS - has the TS Compile been configured?
Looks like the tests are also currently failing:
Test Suites: 1 failed, 5 passed, 6 of 64 total
Tests: 9 failed, 114 passed, 123 total
Snapshots: 0 total
Time: 206.723 s
It appears that the soft-max
tests are receiving a Float32Array
instead of a plain array - should I look where the Float32 is coming from or should I update the unit tests to expect a Floar32Array
instead?
The data returned looks exactly the same, it's just that the object types are different so the deep equality is failing:
Example for one of the tests below:
โ SoftMax โบ .compare2D โบ can run on a simple matrix
assert.deepEqual(received, expected)
Expected value to deeply equal to:
[[-0, 2], [3, 4]]
Received:
[[-0, 2], [3, 4]]
Difference:
- Expected
+ Received
Array [
- Array [
+ Float32Array [
-0,
2,
],
- Array [
+ Float32Array [
3,
4,
],
]
Yes, you can update test to expect Float32Array
.
In the mean-time some tests might still fail, because they have not yet been updated recently. You can can continue working and make sure build process is successful and conversion does not cause increase in no. of failed tests.
Hi! I want to help. If I convert one file, do I need to convert related files too or something? Or just a single file?
You don't need to updated all the related files, just go file by file and make sure build is successful.
Utilities is taken, isnt it?
Activation functions seem like a good place to start, may I work on it?
Utilities is taken, isnt it?
Hi @HarshKhandeparkar, I haven't had a chance to work on this you're welcome to take utilities if you want, just note that needs to be updated above
Np @nabeelvalley. I think I'll work on activation functions :)
@nabeelvalley I think I managed to fix the jest error you were facing in #582. You may copy paste my changes to jest.config.json
or wait for the PR to be merged :)
estimator/ has a single file. Looks like a nice :dart:.
I think I'll snipe some of the utilities next, if you don't mind @nabeelvalley.
You would have probably started in alphabetical order(I am assuming), so I am going to start from the bottom.
Going to snipe utilities/values* tonight :)
Huzzah!
Build systems seems to be broken, I am looking into this issue. ๐งฏ
Can I continue or should I wait?
You should continue your contributions! I will try to fix it asap.
Hi All, taking a look at doing some of these as well today, starting from the top of utilities seeing as @HarshKhandeparkar has started from the bottom
Note, I haven't done utilities/array-lookup-table
as it looks like it's working with some dynamic types and a few different case
s and it's a bit difficult to infer all of these (in my opinion) until recurrent/rnn-time-step
is updated. So doing it after that seems to make sense
I'm also not sure what's going on with the builds/tests as I had some issues from when I pulled the code to begin with, and the errors seem to be inconsistent (I assume due to the order in which tests are running etc.)
I am going to stop targeting utilitites for a while now. Apparently some of them refer to layer
etc. So I am starting with layer/
today. I am going to drop a nuke on it :grin:
Love it!
I'll actually work on utilities/kernel first because it is referenced in layer/base. I'll do base after that.
NOTE: layer/base
is a HOT MESS.
I also wanted to underline that but apparently underline isn't supported on github.
I'm taking over the layer folder from @HarshKhandeparkar (we coordinated already), and will need utilities either done soon, or will need to go ahead and handle:
I'd also eventually like to consolidate these into a single file, but for a later task.
Going to continue working on utilities as well, will update on here as I go:
utilities/to-array
utilities/max
__tests__/utilities/mse.js
test('should return an array if object is passed', () => {
const collection = {
name: 'Steve Jobs',
alive: false,
};
const temp = toArray(collection);
expect(temp.constructor).toBe(Float32Array);
expect(temp.length).toBe(Object.keys(collection).length);
});
@robertleeplummerjr I remember doing utilities/kernel
.
I can take utilities/random
and utilities/random-weight
if no one else wants to take it.
@mubaidr The imports are broken and the tests are failing, am I supposed to ignore all of that and force commit?
@HarshKhandeparkar I have fixed the issue. You can create pull request now.
I'll convert the randos kernel tomorrow :)
I have randos done already, but if younknocknitnout, it all good.
On Thu, Sep 10, 2020, 12:58 PM Harsh Khandeparkar notifications@github.com wrote:
I'll convert the randos kernel tomorrow :)
โ You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BrainJS/brain.js/issues/551#issuecomment-690507706, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFFZO63XW4THPXV4XGVTNLSFEATPANCNFSM4NDROIUQ .
I have randos done already, but if younknocknitnout, it all good.
Oops, sorry but I opened a PR while you were deep asleep ;)
@mubaidr 1) Which file(s) can I work on? 2) is it expected that the many tests fail at the moment?
I am have lots of free time in the next 1-2 weeks and years of experience working with typescript, so if you give me the "go ahead" I can probably convert the whole thing.
@ofirgeller
the type checker and linter shouldn't have any issues though - i think there are just two type checker warnings from what i last saw
@ofirgeller You can select any files from the the repo, and mention it here so that no effort is duplicated. Yes, we do have some failing tests, and we are working on this issue, but as @nabeelvalley said, you can start from utilities and we do have lint
and typescript
scripts (npm run lint
) to help catch errors during conversion process.
Converted "array-lookup-table.js" (short and sweet), moving on to DataFormatter which will take a bit longer.
@ofirgeller I recommend creating a pull request per file (and related changes i.e. imports etc) to make it easier for code review and merge.
@mubaidr If each atomic change is a a separate commit on the same branch, do you still want each commit to be its own PR?
No. multiple commits per PR is good.
@mubaidr and @HarshKhandeparkar, layers are here: https://github.com/BrainJS/brain.js/compare/ts-layers
They are not done, but I've laid the foundation for what they'll look like. I just pushed it, and will merge test into it shortly and resolve conflicts. This includes some pretty heavy upgrades from typescript typings from gpu.js.
Tons more layer conversion, including pool.ts and convolution.ts!
All layers have now been converted. Opened PR here: https://github.com/BrainJS/brain.js/pull/603.
Merged the above. Picking up src/lookup.js
Hi, I'd like to work on this as well. Can you tell me which file is not being worked on?
We plan to gradually migrate brain.js to TypeScript, code base is pretty large, so we would love your help! ๐ช
How to contribute?
Here you can find a guide on how to contribute.
Want to convert something, let us know in the comment and go ahead! ๐
To avoid duplicate work please comment on which part you want to work on (as long as nobody else is working on it) so we can mark it as taken.
Reach out to us! Feel free to reach if you have questions or need help getting started. You can leave comments here or you can tag me in your PR if you need any help or you're not sure about something!
You can also get in touch on our
&
.
Happy Coding! ๐ค
UPDATE:
Wohoooo!!! ๐
All files inside
src
directory are migrated to typescript. (Except few ones which are already taken up and being worked on), though we are still looking on improvements to types in these files and removingany
types from the source. You are welcome to contribute. ๐__tests__
directory has still some files left that needs migration to typescript, so feel free to pick em up! ๐ญUPDATE
For anyone looking to contribute, here is the list of files that still needs typescript migration:
https://github.com/BrainJS/brain.js/search?l=javascript&p=1