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

Add norm API for vectors #207

Merged
merged 3 commits into from
Aug 21, 2024
Merged

Conversation

T45K
Copy link
Contributor

@T45K T45K commented Aug 18, 2024

What

added norm APIs for vector type (i.e., MultiArray<Float, D1> and MultiArray<Double, D1>).
in this implementation, reusing existing norm methods by using origin vector (mk.zeros(size)).

Why

currently, there are norm methods accepting only matrix (i.e., MultiArray<Number, D2>).
however, NumPy provides norm methods for vector.
sample: https://stackoverflow.com/a/43043160

i think it is better to provide the similar API for usability.

Questions

  • in this PR, i added new methods to norm.kt, but should i add them to LinAlgEx.kt file?
  • where should i add tests?

@devcrocod
Copy link
Collaborator

Thank you very much for the changes.

In this PR, I added new methods to norm.kt, but should I add them to the LinAlgEx.kt file?

No, since you are reusing existing implementations, you don't need to add them to LinAlgEx.kt. That file contains methods whose implementations reside in other modules.

Where should I add tests?

Since norm has two implementations, you need to test them in both modules:

In the Kotlin implementation: KELinAlgTest.kt
In the native (OpenBLAS) implementation: NativeLinAlgTest.kt
You don't need to write complex tests. You can use the test from the native implementation as a reference: NativeLinAlgTest.kt, Line 318.

@T45K
Copy link
Contributor Author

T45K commented Aug 19, 2024

@devcrocod
thank you for advice!
i added tests to both of two test classes.

however, unfortunately, i don't have OpenBLAS env on my local.
so, may i ask you to execute tests i added to NativeLinAlgTest instead of me?

@devcrocod
Copy link
Collaborator

All tests have passed.
Thank you very much for your contribution.

I can't say exactly when I will be able to release a new artifact with these changes, as I currently need to completely change the build infrastructure. This is further complicated by issues with the latest versions of Xcode.

@devcrocod devcrocod merged commit b121652 into Kotlin:develop Aug 21, 2024
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.

2 participants