Skip to content
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

[Backport 1.2] Fix innerproduct function to match Dot Product #1126

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

opensearch-trigger-bot[bot]
Copy link

Backport d65b56f from #1004

* Fix innerproduct function to match Dot Product

Signed-off-by: Naarcha-AWS <[email protected]>

* Fix formatting

Signed-off-by: Naarcha-AWS <[email protected]>

* Use dot instead

Signed-off-by: Naarcha-AWS <[email protected]>

* Remove parenthesies

Signed-off-by: Naarcha-AWS <[email protected]>

* Remove space

Signed-off-by: Naarcha-AWS <[email protected]>

Signed-off-by: Naarcha-AWS <[email protected]>
(cherry picked from commit d65b56f)
@opensearch-trigger-bot opensearch-trigger-bot bot requested a review from a team as a code owner September 7, 2022 17:42
JeffHuss
JeffHuss previously approved these changes Sep 8, 2022
@JeffHuss JeffHuss dismissed their stale review September 8, 2022 15:58

Accidentally approved. This shouldn't be merged - it's not correct syntax.

@Naarcha-AWS
Copy link
Collaborator

@JeffH-AWS: Fanit and I agreed it's more understandable without the dot or parentheses included.

@JeffHuss
Copy link

JeffHuss commented Sep 8, 2022

@JeffH-AWS: Fanit and I agreed it's more understandable without the dot or parentheses included.

If that's the case then you need to change it for the other formulas as well. I personally don't think it makes it more readable, and the inconsistency makes it extremely confusing.

@kolchfa-aws
Copy link
Collaborator

The dot notation is implied in multiplication in math, so having a dot there does not add information. We should change this for other formulas that use the same notation. Also, we agreed to create an issue to make all vector notation unified: vectors will be denoted with lowercase bold letters, and scalars will be denoted with lowercase non-bold letters. This is the standard vector notation in math. @Naarcha-AWS - could you create an issue for that or should I? I can take it on when it's created.

@Naarcha-AWS
Copy link
Collaborator

I'll go ahead and create the issue.

@Naarcha-AWS Naarcha-AWS merged commit 25a3632 into 1.2 Sep 8, 2022
@Naarcha-AWS Naarcha-AWS deleted the backport/backport-1004-to-1.2 branch September 14, 2022 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants