Proper error handling of mds-related errors

It is proposed to use mds status flag passed through tdi to check for errors, instead of tests of the returned data directly.

For example replacing

d = tdi('expression')
if ~ischar(d.data) && ~isempty(d.data)
 % use data
else
 warning('problem')
end

with

[d] = tdi('expression')
if ~rem(d.status,2) % unambigious way to check for problems
  warning('problem')
end

Previous discussion:

• @vallar started a discussion: (+10 comments)

@osauter @vallar I continue the discussion here. Also tagging @amerle who may have opinions about this.

• tdi is a class that returns data objects constructed from various mdsvalue calls.
• The status flag is the status flag of mdsvalue that tdi returns. This makes sense and constitutes expected behaviour of the tdi class. I think it is not unreasonable that assume this behaviour stays true. We can add a test if we want to ensure this.

I merely advocate using this existing error handling mechanosm of MDSplus, which is correctly propagated thorugh tdi, to check whether a request for data from MDS fails.

Furthermore we can add of course a further output/error flag to gdat if you want, but that is beyond the scope of this issue. The issue here is how to check, within gdat (or elsewhere for that matter) whether an mds call gave what you expected, to replace things like

d = tdi('expression')
if ~ischar(d.data) && ~isempty(d.data)
 warning('problem')
end

with

[d] = tdi('expression')
if ~rem(d.status,2) % unambigious way to check for problems
  warning('problem')
end

It can be better to use tdi/status but the check will be different from a mdsvalue return. Same for gdat. At some point the data may not come from mds anymore, hence no reason to keep this old convention (not much known actually), but may be 0 for ok and non-zero for not ok would be better...

tdi is very intimately linked to mdsvalue, since it is basically a wrapper that calls it. What I'm proposing is already completely within the mdsplus data retrieval/handling scope of the program.

The fact that the convention is not well known is our fault (for not advertising and documenting it enough within SPC) but that is not a reason not to use it.

changed the description

We had a similar discussion some time ago, it was decided to throw an error (from gdat) if there is an error

Which is useful actually since most users react only if there is an error, but should not be for cases which mean there is no power for example...

To keep in mind

Sorry this comment does not settle the issue.

The issue is whether we should use rem(d.status,2) to check errors in the future in gdat. I think yes. If you think no, and the above reasoning did not convince you, I will drop it and you can close the issue.

Why not check if not 1 instead of recomputing rem() which is done in tdi if I understood correctly?

Because any other odd value also indicates the result is ok. For example [~,s] = mdsopen(61400) gives s=265388041

I don't know enough about mds to know whether mdsdata (which is called in tdi) always returns 1 if correct, but I know that checking odd/even should always work.

but do you know if tdi.status is always 1 iff it's ok. It seems it was analyzing the mds status returned. If we do not know then ok for rem(,2)

For a direct mdsvalue/mdsdata call I would do rem(,2) indeed

For mdsopen I prefer to check the opened shot number is the expected one, but indeed should first check if odd

No, as you can readily see by inspecting the tdi class definition, tdi returns the mds status as-is, except if it catches a further error (it looks for some specific strings in the returned text) in which case it substitutes another error flag -2.

I don't know if checking the shot from mdsopen is always sufficient, I think I remember finding some cases where it was not sufficient. Checking the status flag always works.

OK I have had cases where it opened a previously opened shot, but may be the flag was even I don't know, hence the double check

The status is enough. Otherwise you should also check the name of the tree opened ...

mentioned in merge request !114 (merged)

changed the description

mentioned in merge request !115

While this fix could readily work for calls to the MATLAB tdi class. It wouldn't work in all cases for mdsvalue calls due to some of the error handling by our IT group explicitely replacing the return value with a string without triggering an MDSplus error and thus having an odd status (this is linked to spc/tcv/tbx/mds#8).