Bug in subroutine s_base_set_vect ?

[jaslopes]

Hello,

I think there is a bug in the mentioned subroutine that results in a wrong behaviour operation. If you use

call v%set( vect, first, last )

the code does not allow "last" to be larger than "size( vect )". In my opinion, "last" should not be larger than "first + size( vect ) - 1"

Patch:

@@ -745,8 +745,9 @@ contains integer(psbipk) :: info, first, last, nr

 first_=1

• last_=min(psbsize(x%v),size(val)) if (present(first)) first = max(1,first)

• last_=min(psbsize(x%v),first+size(val)-1) if (present(last)) last = min(last,last)

  if (allocated(x%v)) then

[sfilippone]

The reason for last being no larger than size(vect) is that the assignment statement is x%v(first:last) = vect(1:last-first_+1) Let's consider size(vect)=10, first=2, last=12; this would generate an assignment like x%v(2:12) = vect(1:12-2+1) which would result in out-of-bounds (because of the way the dummy argument vect is declared, its first index is always 1). Did I understand your intention correctly?

The real question though is what would be a good interface. This interface was really thought of a way to set a local portion of an encapsulated vector in the use case where I have the local points available in an auxiliary vector VAL but they run from 1 to desc_data%get_nrows(), whereas the internal component of X runs from 1 to desc_data%get_ncols() which is larger. THerefore the most common use case would be call x%set(vect,last=n_rows) and I think I put in the FIRST argument just for simmetry, but never expecting to have much use for it. If you have alternative suggestions please let me know.

S.

On Wed, Sep 12, 2018 at 5:19 PM Alexandre Silva Lopes < notifications@github.com> wrote:

I think there is a bug in the mentioned subroutine that results in a wrong behaviour operation. If you use

call v%set( vect, first, last )

the code does not allow "last" to be larger than "size( vect )". In my opinion, "last" should not be larger than "first + size( vect ) - 1"

Patch:

@@ -745,8 +745,9 @@ contains integer(psbipk) :: info, first, last, nr

first_=1

• last_=min(psbsize(x%v),size(val)) if (present(first)) first = max(1,first)

• last_=min(psbsize(x%v),first+size(val)-1) if (present(last)) last = min(last,last)

  if (allocated(x%v)) then

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sfilippone/psblas3/issues/9, or mute the thread https://github.com/notifications/unsubscribe-auth/APX-TzYDax0mrJJWHkzhv1HuMvqRkQczks5uaTQfgaJpZM4Wlu_v .

[jaslopes]

But if you still consider size(vect)=10, first=21, last=30, I think this would generate an assignment x%v(21:30) = vect(1:30-21+1) which is valid.

However, currently it generates an assignment x%v(21:10) = vect(1:10-21+1) which does nothing, because of the line last_=min(psb_size(x%v),size(val))

That's why I think that you should use last_=min(psbsize(x%v),first+size(val)-1)

My problem was that I was copying blocks from a vector of size 'n' and as soon as 'first' becomes larger than 'n' (but last <= first + n - 1), nothing is copied.

[sfilippone]

Hmm. Looks like you're right and I was wrong (possibly due to insufficient amount of coffee this morning ;-) ) On Thu, Sep 13, 2018 at 8:44 AM Alexandre Silva Lopes notifications@github.com wrote:

But if you still consider size(vect)=10, first=21, last=30, I think this would generate an assignment x%v(21:30) = vect(1:30-21+1) which is valid.

However, currently it generates an assignment x%v(21:10) = vect(1:10-21+1) which does nothing, because of the line last_=min(psb_size(x%v),size(val))

That's why I think that you should use last_=min(psbsize(x%v),first+size(val)-1)

My problem was that I was copying blocks from a vector of size 'n' and as soon as 'first' becomes larger than 'n' (but last <= first + n - 1), nothing is copied.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[sfilippone]

Fixed on development at https://github.com/sfilippone/psblas3/commit/1a5ee8c46a4a332fce997522560d7b9f6dbde547

[jaslopes]

Thanks!