feat: update loops and add publish workflows #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
adithyaakrishna
commented
Jun 3, 2024
Comment on lines
-80
to
+90
| passages.forEach((item) => { | ||
| const match = item.match(/\[(\d+)\]/g); | ||
| for (let i = 0; i < passages.length; i++) { | ||
| const match = passages[i].match(/\[(\d+)\]/g); | ||
| if (match) { | ||
| const index = parseInt(match[0].replace(/\[|\]/g, "")); | ||
| result.push(list[index - 1][idKey]); | ||
| const index = parseInt(match[0].replace(/\[|\]/g, ""), 10); | ||
| if (index > 0 && index <= list.length) { | ||
| // Check to ensure index is within bounds to prevent index -1/out of bounds error. | ||
| result.push(list[index - 1][idKey]); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
- Added a check to ensure that the index derived from the match is within the bounds of the list array
- Using
foras it recdces the overhead of calling a function for each item in passages.
adithyaakrishna
commented
Jun 3, 2024
Comment on lines
-11
to
+20
| rankLists.forEach((list) => { | ||
| list.forEach((item, index) => { | ||
| const rrfScore = 1 / (c + index + 1); // index is zero-based, add 1 for correct rank | ||
| const currentScore = scores.get(item[idKey] as string) || 0; | ||
| scores.set(item[idKey] as string, currentScore + rrfScore); | ||
| }); | ||
| }); | ||
| for (let i = 0; i < rankLists.length; i++) { | ||
| const list = rankLists[i]; | ||
| for (let index = 0; index < list.length; index++) { | ||
| const item = list[index]; | ||
| const itemId = item[idKey] as string; // Cast once and reuse | ||
| const rrfScore = 1 / (c + index + 1); // Compute RRF score | ||
| const currentScore = scores.get(itemId) || 0; // Get current score, default to 0 | ||
| scores.set(itemId, currentScore + rrfScore); // Update score | ||
| } | ||
| } |
There was a problem hiding this comment.
- The conversion item[idKey] as string is done twice per item, which is redundant, so IMO we can compute it once and reuse it 😶
- Minimize map accesses as reducing the amount of ops can give more perf. benefits
- Used
forloops here as well instead offorEach
adithyaakrishna
commented
Jun 3, 2024
Comment on lines
-127
to
-132
| ranks.forEach((rank) => { | ||
| let match = batch.filter((item) => item.startsWith(rank)); | ||
| if (match.length > 0) { | ||
| result.push(match[0]); | ||
| for (let i = 0; i < ranks.length; i++) { | ||
| const rank = ranks[i]; | ||
| let matchFound = false; | ||
| for (let j = 0; j < batch.length; j++) { | ||
| if (batch[j].startsWith(rank)) { | ||
| result.push(batch[j]); | ||
| matchFound = true; | ||
| break; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
- We can avoid using filter inside a for loop as it iterates over the entire
batchfor eachrank - Used
breakto exit early in case so that we prevent unnecessary iterations
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
lucasjacks0n
approved these changes
Jun 3, 2024
|
Thanks @lucasjacks0n :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
npm publishworkflow (Needs NPM Secret to be added to settings).envfor API Keys/Secrets