visit icon indicating copy to clipboard operation
visit copied to clipboard

how to test if a query failed in the cli?

Open cyrush opened this issue 1 year ago • 11 comments

Describe the bug

Query() returns the result of GetQueryOutputString(), how do we know if a query failed?

In the case of an error, (at least for the XRayQuery) errors are prefixed with engine_, where normal output lacks this string.

>>> v = Query("XRay Image", "hdf5", ".", 1, 0.0, 2.5, 10.0, 0, 0, 10., 10., 300, 300, ("d", "p"))
>>> # no output ....
>>> print(v)
engine_par: Variables d and p resulted in no data being selected.

That prefix is an embarrassing way to see if something went wrong, is there a better way?

To Reproduce

Do an xray image query with bogus params as above on any dataset.

Desktop

  • OS and version: macOS monterey
  • VisIt Version: 3.3RC
  • Server info (if applicable):

cyrush avatar Mar 10 '23 17:03 cyrush

We should have a discussion about the best way to implement this.

brugger1 avatar Mar 16 '23 16:03 brugger1

The function GetQueryOutputValue now returns None if the query returns no values. This could be used to determine if an error occurred. This won't work in the case of the "XRay Image" query since it always returns no values and hence None for GetQueryOutputValue. It could be changed to return no values if the query fails and the number of files generated in the case of success. In general, all the queries would need to be checked to ensure that they return no values in the case of an error for this to be universally applicable.

I'm taking this out of the reviewed state so we can discuss it again.

brugger1 avatar Aug 04 '23 17:08 brugger1

I would think GetQueryOutputValue for the XRay Image query could return either a list of the names of the files it produced or, a list of the Silo variables and/or meshes it produced (in the one and only Silo file...if that is how the most recent implementation behaves). If it fails, it produces Py_None in GetQueryOutputValue or maybe a list of size zero. So, just a swizzle on @brugger1 idea but is more specifically indicating what the outputs actually where.

markcmiller86 avatar Aug 04 '23 17:08 markcmiller86

Question: The X Ray Image Query is an avtDatasetQuery for which we have the following members:

std::string              resMsg;
doubleVector             resValue;
std::string              xmlResult;

Here is the function signature for setting the result values: virtual void SetResultValue(const double &d, const int i = 0); and virtual void SetResultValues(const doubleVector &d)

How can I make the query return strings such that GetQueryOutputValue will yield a list of filenames or something? As opposed to a vector of doubles.

Or is that even necessary? For context, you already get the filenames out from the result message that is set.

JustinPrivitera avatar Aug 07 '23 18:08 JustinPrivitera

My prior guidance was of course assuming a query result holding a list of strings was possible to return from a Quey. Maybe it isn't.

markcmiller86 avatar Aug 07 '23 20:08 markcmiller86

Since the XRayImage query is a different kind of query, not sure you need to have GetQueryOutputValue return anything. If we agree it's not necessary, the fact that GetQueryOutputValue isn't valid for an XRayImageQuery should be added to documentation.

If it's decided GetQueryOutputValue should be populated for the XRayImageQuery, the best option would be to create a stringVector var similar to the doubleVector and have the XRayImageQuery populate it. GetQueryOutputValue could then test for and non-empty stringvector if the doubleVector is empty. Cannot recall off the top of my head if this would need to be duplicated in the QueryAttributes as well.

Perhaps Queries in general should be changed to return success or failure, then the user can retrieve the results from whichever output makes sense when the Query succeeds (or the message when they fail). Would be a big change from the current modality but it makes more sense than trying to figure out from the output values whether the query succeeded or not. Would also have to do away with the 'SetQueryOutputToXXX' methods that change what gets returned based on user preference.

biagas avatar Aug 07 '23 20:08 biagas

@biagas what has been the current practice in user's CLI code to determine when a query has succeeded or failed? And, what, if anything differently, do you think users ought to be doing for that instead?

As far as some parts of the CLI go (e.g. those involved in assigning values to various VisIt object's or their members), we use exceptions to indicate erroroneous coding. We do this because a) we want the error detected as soon as practical, and b) don't want inadvertent mistakes in building a VisIt object to wind up going sliently ignored and c) there is no option other than exceptions given a & b.

In other parts of the CLI, I am honestly not sure what the current practice or preferred method of indicating error to the calling program is. Do we favor return values or exceptions? For example if SetPlotSILRestriction(...) fails, how is this indicated to the caller? For example in PySILRestrictionBase.C, I see something like...

static PyObject *
SILRestriction_TurnOnAll(PyObject *self, PyObject *args)
{           
    TRY     
    {       
        PySILRestrictionObject *obj = (PySILRestrictionObject *)self;
        avtSILRestriction_p silr = *(obj->silr);
        silr->TurnOnAll();
    }       
    CATCH2(VisItException, e)
    {
        CATCH_RETURN2(1, PyErr_Format(PyExc_RuntimeError, "Internal VisIt exception \"%s\"", e.Message().c_str()));
    }   
    ENDTRY
    
    Py_INCREF(Py_None);
    return Py_None;

which seems to catch VisIt exceptions and turn them into Python exceptions and otherwise (success) returns Py_None.

I think this is potentially a deliberation topic and one probably @cyrush has highly informed opinions on.

markcmiller86 avatar Aug 07 '23 20:08 markcmiller86

what has been the current practice in user's CLI code to determine when a query has succeeded or failed?

I'm not sure there is any current practice or standard.

For the x ray query, we don't want a failed query to halt a user's program, so they can test if queries fail by looking at the result message from them. Not a great solution.

I think this is potentially a deliberation topic and one probably @cyrush has highly informed opinions on.

+1

JustinPrivitera avatar Aug 07 '23 22:08 JustinPrivitera

Perhaps the answer for the X Ray Query is to pack a mapnode into xml like so: https://github.com/visit-dav/visit/blob/8db39767c1e5de0be3bf4b93fe91a91a7d1fc6c4/src/avt/Queries/Queries/avtConnComponentsSummaryQuery.C#L383

Into the mapnode we can put a list of the filenames.

Then the user can use GetQueryOutputObject to get out a python list of strings containing the filenames.

JustinPrivitera avatar Aug 12 '23 01:08 JustinPrivitera

Query should return 0 or 1 depending on success or failure of the query.

brugger1 avatar Aug 22 '23 23:08 brugger1

#19011 for the x ray query

JustinPrivitera avatar Oct 31 '23 20:10 JustinPrivitera