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

Made changes to allow to work with large datasets #32

Closed

Conversation

LeonidBeynenson
Copy link

Earlier sometimes error happened when a temporary buffer in ml module were created as a Mat,
since the length of the row of the buffer (in bytes) was sizeof(int) * (work_var_count + 1)*sample_count
-- in some cases it was greater than INT_MAX.

Earlier sometimes error happened when temporary buffers in ml module were created as a Mat,
since the length of the row of the buffer (in bytes) was sizeof(int) * (work_var_count + 1)*sample_count
-- in some cases it was greater than INT_MAX.
@ghost ghost assigned mdim Oct 29, 2012
@vpisarev
Copy link
Contributor

ok, just documenting what we discussed with Leonid.
The patch includes quite many changes and most of the changes replace the same simple formula with the same more complex formula. It's not guaranteed that:

  1. all the places are modified.
  2. the complex formula will work fine in future (and in this case we will need to make it even more complex in all those places?!)

So, instead I suggest to replace "CvMat* buf" with "<some_object_t> but", which will have the proper methods. It should solve both problems:

  1. If we forget to replace one of "buf->cols" etc., we get compile error.
  2. the complex formula will be hidden in the method and therefore we don't have to duplicate it in many places.
    We can do such a change because the patch is for master, not 2.4, and in 2.5 we can break binary compatibility.

@kirill-korniakov
Copy link

Leonid, any plans with this task? Could you please share your thoughts?

@LeonidBeynenson
Copy link
Author

I can start work on this task, but I thought that now Vadim Pisarevsky is going to work with it.

@ghost ghost assigned vpisarev Dec 10, 2012
@alekcac
Copy link
Contributor

alekcac commented Dec 12, 2012

Vadim, any changes?

@kirill-korniakov
Copy link

I suppose that Leonid will finish this task. Leonid, am I right?

@LeonidBeynenson
Copy link
Author

I will finish it when I have a time slot for it.

@mdim
Copy link
Contributor

mdim commented Jan 21, 2013

@kirill-kornyakov Kirill, when you give Leonid a time slot for this issue eventually? :)
@LeonidBeynenson Leonid, please back to this old task!

@vpisarev
Copy link
Contributor

vpisarev commented Feb 4, 2013

closing this one, because it's superseded by #395

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.

5 participants