Implemented normalizeInPlace. #207
Conversation
|
|
||
| @Override | ||
| public void normalizeInPlace(VectorAccumulator acc) { | ||
| Vector normalized = normalize(acc); |
There was a problem hiding this comment.
It is not really in-place, since you created a vector here.
There was a problem hiding this comment.
I did not realize that you wanted no extra object creation done.
I look into removing this extra Vector creation and if I get it I will amend the commit.
|
Hi @DWiechert! Thanks for the PR. The la4j is currently in the middle of epic journey that involves this If you will to contribute more - let me know I will find something that is important right now. |
|
Hey there @vkostyukov. I'd be happy to try and tackle some other issues. I only submitted this PR because both this method and I'm still trying to poke through and learn this library at the moment, but I'll do my best to try and help out. |
|
Sure! Here is the issue that doesn't conflict with operations redesign: #202. I will update the description of that ticket to make it clear what to do. Feel free to take it and ask any questions there. |
|
Ok, thanks. As for this PR, what should be done? Will it just be left here until that Operations work is done and re-visited, or is there a way to close it out? |
|
I think it would be better to close it for now. The in-place operations are part of 0.6.0 milestone (the current one is 0.5.0). So we would probably don't want it handing here for a bunch of months. Let's close it to keep the repository clear. |
Based on the comments on #206, I added a default
normalizeInPlace()using the Euclidean accumulator as well asnormalizeInPlace(VectorAccumulator acc).