mdsplus icon indicating copy to clipboard operation
mdsplus copied to clipboard

SUM no longer ignores ROPRAND

Open merlea opened this issue 2 years ago • 2 comments

$ROPRAND is no longer ignored in SUM operations while documentation still says it should be ignored.

TDI> SUM([1.,$ROPRAND]) $ROPRAND

while in older versions (e.g. stable_release-7-1-10) TDI> SUM([1.,$ROPRAND]) 1.

merlea avatar Mar 17 '22 13:03 merlea

I am surprised that it used to return 1. Can you point at where it says it should? It originally was defined as $ROPRAND op anything == $ROPRAND

On Mar 17, 2022, at 9:31 AM, Antoine Merle @.***> wrote:

$ROPRAND is no longer ignored in SUM operations while documentation https://www.mdsplus.org/index.php/Documentation:Reference:TDI_S2#sum still says it should be ignored.

TDI> SUM([1.,$ROPRAND]) $ROPRAND

while in older versions (e.g. stable_release-7-1-10) TDI> SUM([1.,$ROPRAND]) 1.

— Reply to this email directly, view it on GitHub https://github.com/MDSplus/mdsplus/issues/2454, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABY5AZKNR5QHPKZTGQ7XOFLVAMXZXANCNFSM5Q67PAMQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you are subscribed to this thread.

joshStillerman avatar Mar 17 '22 14:03 joshStillerman

The link above (pointing to: https://www.mdsplus.org/index.php/Documentation:Reference:TDI_S2#sum).

merlea avatar Mar 17 '22 15:03 merlea

It looks to me that the tdishr function Tdi3Sum() hasn't changed since its inception. I couldn't see any changes to it in GitHub history. It seems to return 1 only if none of the case DTYPE _x statements are not satisfied. I will investigate further.

santorofer avatar Mar 06 '23 22:03 santorofer

The documentation is confusing, but $ROPRAND being returned is correct. Please suggest an edit to the documentation, and we'll do the best we can.

WhoBrokeTheBuild avatar Mar 29 '23 19:03 WhoBrokeTheBuild

I might not be a native english speaker but (quoting the current documentation)

The result without DIM is the sum of the elements of ARRAY, using only those with true MASK values and value not equal to the reserved operand ($ROPRAND)

seems to express clearly that $ROPRAND values will be ignored which was consistent with the behaviour of stable_release-7.1.10.

That said I understand that there are now 2 historical behaviours and only one can survive.

For the documentation I would suggest removing this last part of the sentence meaning replacing the first sentence with:

The result without DIM is the sum of the elements of
ARRAY, using only those with true MASK values.

I would then add another paragraph saying

Reserved operand ($ROPRAND) values are included. This behaviour
has changed and in releases prior to stable release XXX and alpha
release YYY $ROPRAND values were ignored. To ignore $ROPRAND
values replace ARRAY with FIX_ROPRAND(ARRAY,0).

Also the sentence If no value is found, 1 is given. needs to be corrected as 0 is returned in that case.

I would also advise you to check similar functions (e.g. PRODUCT) and make the documentation more uniform.

merlea avatar Mar 30 '23 08:03 merlea

Thank you for this very clear suggestion. We will update the documentation

Since I did not track down when this behavior changed, and as Stephen says the code has not changed, I did not put in the stuff about 'behavior has changed'

joshStillerman avatar Mar 30 '23 21:03 joshStillerman