-
Notifications
You must be signed in to change notification settings - Fork 8
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
1.0.5: fix matrix operations and size
- Loading branch information
1 parent
103a629
commit 872966f
Showing
2 changed files
with
93 additions
and
100 deletions.
There are no files selected for viewing
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
Oops, something went wrong.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dr-ni .
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why all those massive changes?
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dr-ni .
Because in the original version, all arrays are assigned size N * N, where N is the number of points. And this is completely wrong. Most matrices have a work area of either N * 3 or 3 * 3. What for the rest of the memory allocated?
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ineffective for sure, but not wrong. I'll see when I'll find time for testing
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dr-ni
No. This is definitely wrong.
PS: I'm trying to "get" the test data. The silence presently.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bigger matrix multiplication with 0 values is like the correct allocated matrix multiplication
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni say:
But what for?
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's for sure ineffective. Maybe I've forgotten to change this after my initial tests.
The advantage is not to test a matrix dimension mismatch every time.
So maybe it's now more false than before.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , I have exactly the same opinion.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please revert
old:
new:
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , ok.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's actually better to leave it like it is because so many tests have been already made.
It's better to check huge data sets if it's consuming an unacceptable amount of memory.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , you're a liar.
1.0.4:
1.0.5:
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm maybe on Raspberry there are some issues with valgrind
I've made sthe tests on different machines
I'll test it now on the same hardware
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pi@rp4-sams:~/helmert3d-o $ valgrind helmparms3d examples/testpoints_src.txt examples/testpoints_dest.txt
==6191== Memcheck, a memory error detector
==6191== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==6191== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==6191== Command: helmparms3d examples/testpoints_src.txt examples/testpoints_dest.txt
==6191==
ok so valgrind is not working correct on my pi here
please do not reproach in the chat, that is rude (liar)
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , for test https://github.com/Geo-Linux-Calculations/helmparms3d
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you'll find some time you can also do some tests with splint
if this will pass you can merge again...
Since valgrind is ok it seems be looking good
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , i am now interested in something else.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does the old version show?
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , that is what it shows. The new one is the same. Most warnings can be safely ignored, but you need to study it.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this?
Hi dr-ni,
Your invitation to the Geo Linux Calculations organization on GitHub has been canceled. Links to the invitation in previous emails will no longer work.
Thanks,
The GitHub Team
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @dr-ni , i changed my mind. No more invitations will come. Do not mind it.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean?
shall I invite you again?
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, @dr-ni . Your invitations have nothing to do with it. These were my invitations of GLC.ORG.
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok,
if splint is the same for both I think we can merge again
can you see a difference in memory usage?
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , with such minor data, it is impossible to see some difference. But so far no one has sent any significant projects. If you have a forum on which you can ask these projects, do it.
Ask:
NameTool - XYZsource - XYZdestination - XYZtrans - XYZcalculation
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's merge again we'll see if there is an issue, soon, I think
872966f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-ni , i can just restore the commit. Borrow it from GLC.ORG. But is it worth it?