Possible bug in C++ MDSobjects interface.

[merlea]

Hello,

I came across what seems like a bug in the C++ implementation when reading nodes containing expressions. I tried to reduce the case to something workable and testable.

Let's create a tree with the Tcl script:

edit test/new
add node equilibrium/usage=structure
set def .equilibrium
add node time/usage=signal
add node data/usage=signal
write
close
quit

and populate it using C++

#include <mdsobjects.h>
#include <string>

int main(void)
{

  int shot = 1;

  std::cout << "STEP 1: Fill tree" << std::endl;

  MDSplus::Tree * modelTree = new MDSplus::Tree("test", -1);
  modelTree->createPulse(shot);
  delete modelTree;
  MDSplus::Tree * tree = new MDSplus::Tree("test", shot);
  tree->setDefault(tree->getNode("\\TOP"));

  {
    MDSplus::TreeNode * node = tree->getNode("\\TOP.EQUILIBRIUM:DATA");
    node->deleteData();
    char bufd[] = "data(GetSegment(build_path(\'\\\\TOP.EQUILIBRIUM:TIME\'),0))[0]";
    MDSplus::Data * dataData = MDSplus::compile(bufd,tree);
    node->putData(dataData);
    delete node;
    MDSplus::deleteData(dataData);

    node = tree->getNode("\\TOP.EQUILIBRIUM:TIME");
    node->deleteData();
    char sbuft[] = "0";
    MDSplus::Data * startData = MDSplus::compile(sbuft,tree);
    char ebuft[] = "2";
    MDSplus::Data * endData = MDSplus::compile(ebuft,tree);
    char dbuft[] = "[0 : 2]";
    MDSplus::Data * dimData = MDSplus::compile(dbuft,tree);
    double arrayt[3] = {1.,2.,3.};
    int dimt[1] = {3};
    MDSplus::Float64Array * timeData = new MDSplus::Float64Array(arrayt,1,dimt);
    node->makeSegment(startData,endData,dimData,timeData);
    delete node;
    MDSplus::deleteData(startData);
    MDSplus::deleteData(endData);
    MDSplus::deleteData(dimData);
    MDSplus::deleteData(timeData);
  }

  delete tree;
}

then try to read it using C++

#include <mdsobjects.h>
#include <string>

int main(void)
{

  int shot = 1;

  std::cout << "STEP 2: Standard read of segment limits" << std::endl;

  MDSplus::Tree * tree = new MDSplus::Tree("test",shot);
  tree->setDefault(tree->getNode("\\TOP"));

  {
    MDSplus::TreeNode * node = tree->getNode("\\TOP.EQUILIBRIUM:DATA");
    MDSplus::Data *data = node->getData();

    std::cout << "  Decompiled expression for data" << std::endl;
    std::cout << data->decompile() << std::endl;

    MDSplus::Data * start = NULL;
    try {
      start = data->data();
      std::cout << "data value: " << start->getString() << std::endl;
    } catch(std::exception &exc) {
      std::cout << "Caught exception while extracting data:" <<std::endl;
      std::cout << "\t" << exc.what() << std::endl;
    }

    if (start) MDSplus::deleteData(start);
    MDSplus::deleteData(data);
   }

  delete tree;
}

I get this output:

STEP 2: Standard read of segment limits
  Decompiled expression for data
DATA(GetSegment(\TOP.EQUILIBRIUM:TIME, 0))[0]
Caught exception while extracting data:
        %TREE-W-NOT_OPEN, Tree not currently open

If I now try to compile the expression again, as in

#include <mdsobjects.h>
#include <string>

int main(void)
{

  int shot = 1;

  std::cout << "STEP 3: Read of segment limits with decompile/compile" << std::endl;

  MDSplus::Tree * tree = new MDSplus::Tree("test",shot);
  tree->setDefault(tree->getNode("\\TOP"));

  {
    MDSplus::TreeNode * node = tree->getNode("\\TOP.EQUILIBRIUM:DATA");
    MDSplus::Data *data, *data2;
    data = node->getData();
    data2 = MDSplus::compile(data->decompile(),tree);

    std::cout << "  Decompiled expression for data" << std::endl;
    std::cout << data2->decompile() << std::endl;

    MDSplus::Data * start = NULL;
    try {
      start = data2->data();
      std::cout << "data value: " << start->getString() << std::endl;
    } catch(std::exception &exc) {
      std::cout << "Caught exception while extracting data" <<std::endl;
      std::cout << "\t" << exc.what() << std::endl;
    }

    MDSplus::deleteData(data);
    if(start) MDSplus::deleteData(start);
    MDSplus::deleteData(data2);
   }

  delete tree;
}

I get this output:

STEP 3: Read of segment limits with decompile/compile
  Decompiled expression for data
DATA(GetSegment(.EQUILIBRIUM:TIME, 0))[0]
data value: 1D0

which is the expected answer.

As you can see the decompiled expressions are slightly different:

DATA(GetSegment(\TOP.EQUILIBRIUM:TIME, 0))[0]
DATA(GetSegment(.EQUILIBRIUM:TIME, 0))[0]

which is a sign of the problem rather than its source I believe.

If I however use python or tdic to read the data, I always get the expected anser and the decompiled expression is displayed as:

DATA(GetSegment(.EQUILIBRIUM:TIME, 0))[0]

I tried several MDSplus versions including stable_release-7-46-1, stable and alpha and all give the same answer.

Could you check if you get the same behavior and if this is a bug or a user problem?

Thanks, Antoine.

[GabrieleManduchi]

I suspect this is related in some way to the fact that in the compiled expression a BUILD_PATH descriptor is present. I will investigate this week.

                                            Gabriele

On 05/07/2019 15:31, Antoine Merle wrote:

Hello,

I came across what seems like a bug in the C++ implementation when reading nodes containing expressions. I tried to reduce the case to something workable and testable.

Let's create a tree with the Tcl script:

|edit test/new add node equilibrium/usage=structure set def .equilibrium add node time/usage=signal add node data/usage=signal write close quit |

and populate it using C++

|#include #include int main(void) { int shot = 1; std::cout << "STEP 1: Fill tree" << std::endl; MDSplus::Tree modelTree = new MDSplus::Tree("test", -1); modelTree->createPulse(shot); delete modelTree; MDSplus::Tree tree = new MDSplus::Tree("test", shot); tree->setDefault(tree->getNode("\TOP")); { MDSplus::TreeNode node = tree->getNode("\TOP.EQUILIBRIUM:DATA"); node->deleteData(); char bufd[] = "data(GetSegment(build_path(\'\\TOP.EQUILIBRIUM:TIME\'),0))[0]"; MDSplus::Data dataData = MDSplus::compile(bufd,tree); node->putData(dataData); delete node; MDSplus::deleteData(dataData); node = tree->getNode("\TOP.EQUILIBRIUM:TIME"); node->deleteData(); char sbuft[] = "0"; MDSplus::Data startData = MDSplus::compile(sbuft,tree); char ebuft[] = "2"; MDSplus::Data endData = MDSplus::compile(ebuft,tree); char dbuft[] = "[0 : 2]"; MDSplus::Data dimData = MDSplus::compile(dbuft,tree); double arrayt[3] = {1.,2.,3.}; int dimt[1] = {3}; MDSplus::Float64Array timeData = new MDSplus::Float64Array(arrayt,1,dimt); node->makeSegment(startData,endData,dimData,timeData); delete node; MDSplus::deleteData(startData); MDSplus::deleteData(endData); MDSplus::deleteData(dimData); MDSplus::deleteData(timeData); } delete tree; } |

then try to read it using C++

|#include #include int main(void) { int shot = 1; std::cout << "STEP 2: Standard read of segment limits" << std::endl; MDSplus::Tree tree = new MDSplus::Tree("test",shot); tree->setDefault(tree->getNode("\TOP")); { MDSplus::TreeNode node = tree->getNode("\TOP.EQUILIBRIUM:DATA"); MDSplus::Data *data = node->getData(); std::cout << " Decompiled expression for data" << std::endl; std::cout << data->decompile() << std::endl; MDSplus::Data

• start = NULL; try { start = data->data(); std::cout << "data value: " << start->getString() << std::endl; } catch(std::exception &exc) { std::cout << "Caught exception while extracting data:" <<std::endl; std::cout << "\t" << exc.what() << std::endl; } if (start) MDSplus::deleteData(start); MDSplus::deleteData(data); } delete tree; } |

I get this output:

|STEP 2: Standard read of segment limits Decompiled expression for data DATA(GetSegment(\TOP.EQUILIBRIUM:TIME, 0))[0] Caught exception while extracting data: %TREE-W-NOT_OPEN, Tree not currently open |

If I now try to compile the expression again, as in

|#include #include int main(void) { int shot = 1; std::cout << "STEP 3: Read of segment limits with decompile/compile" << std::endl; MDSplus::Tree tree = new MDSplus::Tree("test",shot); tree->setDefault(tree->getNode("\TOP")); { MDSplus::TreeNode node = tree->getNode("\TOP.EQUILIBRIUM:DATA"); MDSplus::Data data, data2; data = node->getData(); data2 = MDSplus::compile(data->decompile(),tree); std::cout << " Decompiled expression for data" << std::endl; std::cout << data2->decompile() << std::endl; MDSplus::Data * start = NULL; try { start = data2->data(); std::cout << "data value: " << start->getString() << std::endl; } catch(std::exception &exc) { std::cout << "Caught exception while extracting data" <<std::endl; std::cout << "\t" << exc.what() << std::endl; } MDSplus::deleteData(data); if(start) MDSplus::deleteData(start); MDSplus::deleteData(data2); } delete tree; } |

I get this output:

|STEP 3: Read of segment limits with decompile/compile Decompiled expression for data DATA(GetSegment(.EQUILIBRIUM:TIME, 0))[0] data value: 1D0 |

which is the expected answer.

As you can see the decompiled expressions are slightly different:

|DATA(GetSegment(\TOP.EQUILIBRIUM:TIME, 0))[0] DATA(GetSegment(.EQUILIBRIUM:TIME, 0))[0] |

which is a sign of the problem rather than its source I believe.

If I however use python or tdic to read the data, I always get the expected anser and the decompiled expression is displayed as: DATA(GetSegment(.EQUILIBRIUM:TIME, 0))[0]

I tried several MDSplus versions including stable_release-7-46-1, stable and alpha and all give the same answer.

Could you check if you get the same behavior and if this is a bug or a user problem?

Thanks, Antoine.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MDSplus/mdsplus/issues/1737?email_source=notifications&email_token=ACCLESDYWTMM7TYEHQVU35LP55EKFA5CNFSM4H6LKXOKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G5R4HZA, or mute the thread https://github.com/notifications/unsubscribe-auth/ACCLESDGJD2QZJPSRMY2KOLP55EKFANCNFSM4H6LKXOA.

-- Gabriele Manduchi

Istituto Gas Ionizzati del CNR Consorzio RFX - Associazione EURATOM/ENEA sulla Fusione Corso Stati Uniti 4, 35127 Padova - Italy ph +39-049-829-5039/-5000 fax +39-049-8700718 mailto:gabriele.manduchi@igi.cnr.it, http://www.igi.cnr.it

[GabrieleManduchi]

Finally I discovered what happens...not trivial.....

The reason of this behavior is that in test tree the expression data(GetSegment(build_path(\'\\TOP.EQUILIBRIUM:TIME\'),0))[0] is saved. This expression contains a reference to a tree node (\TOP.EQUILIBRIUM:TIME). When that node is read via getData(), the expression is rebuilt in memory. When finally it is evaluated via data() method, MDSplus finds the path, but it has no way to know to what tree it refers, and therefore issues the exception. The reason why in the example where you call first compile() no exception is issued is that you passed a tree object to compile() (it uses this tree reference in order so solve possible tree node references) that made it the default tree, so that when data() is called afterwards is succeeds in evaluating the path since there is in this case a default tree.

In conclusion: it suffice calling first setTree(tree) before evaluating an expression that contains references to tree nodes so that MDSplus knows from there taking the node content. This is the amended example:

include

include

int main(void) {

int shot = 1;

std::cout << "STEP 2: Standard read of segment limits" << std::endl;

MDSplus::Tree * tree = new MDSplus::Tree("test",shot); tree->setDefault(tree->getNode("\TOP"));

{ MDSplus::TreeNode node = tree->getNode("\TOP.EQUILIBRIUM:DATA"); MDSplus::Data data = node->getData();

std::cout << "  Decompiled expression for data" << std::endl;
std::cout << data->decompile() << std::endl;

MDSplus::Data * start = NULL;
try {
  **setActiveTree(tree);**
  start = data->data();
  std::cout << "data value: " << start->getString() << std::endl;
} catch(std::exception &exc) {
  std::cout << "Caught exception while extracting data:" <<std::endl;
  std::cout << "\t" << exc.what() << std::endl;
}

if (start) MDSplus::deleteData(start);
MDSplus::deleteData(data);

}

delete tree; }

Let me know if this solves your problem and in case I will close the issue.

[merlea]

Hi Gabriele,

Thank you for looking into this. Yet this does solve the problem.

However there seems to be a difference between the C++ interface and the python one (and also tdic) for this matter). For example in Python the following script works fine:

#!/usr/bin/env python

import sys, os
try:
    path = sys.path
    sys.path.append(os.getenv('MDSPLUS_DIR')+"/mdsobjects")
    import python as MDSplus
except:
    sys.path = path
    sys.path.append(os.getenv('MDSPLUS_DIR')+"/python")
    import MDSplus

shot = 1

print("STEP 2: Standard read of segment limits")

tree = MDSplus.Tree(tree="test",shot=shot)
tree.setDefault(tree.getNode("\\TOP"))
node = tree.getNode("\\TOP.EQUILIBRIUM:DATA")
data = node.getData()

print("  Decompiled expression for data")
print(data.decompile())

try:
    start = data.getData();
    print("data value: "+str(start))
except Exception as exc:
    print("Caught exception while extracting data:")
    print("\t"+str(exc))

Note that we are not re-setting the active tree. I guess Python (and tdic) are using the last tree that was opened. In the case of Python I can also see that the start variable has an attribute called "tree", so maybe it is picked up from there ... Does creating a new Tree object implies making this the active tree in C++ ?

Antoine.

[GabrieleManduchi]

Looking at the source code, I see that the line that makes the tree just instantiated as the default one has been commented, and now I remember why. Personally I prefer to be more explicit in the definition of the active tree. MDSplus originally used a static definition of the trees, but now it allows running at the same time on different tree in a thread safe way. For this reason, the data objects normally bring also a reference ti the tree they refer to. However, there are a few situation in which the concept of the active tree is still necessary, such as compiling or evaluating an expression that contains path references.

Just to give you an example of a potential risk of default trees, it may happen that an expression containing a node path that is legal under the currently active tree (without the user being aware) but that is different from the tree the user had in mind when building the expression. In this case compilation would resolve that path into a nid (node indentifier), that is NOT the right one, creating a situation very hard to debug. This why I preferred to avoid default tree unless really required, but my view could be a bit different from that of the python developers.

[tfredian]

That is exactly why the Tree instances in python have the tdiCompile, tdiExecute and tdiEvaluate methods which use the tree context of the Tree object instance. Perhaps the c++ Tree object should have these as well.

[GabrieleManduchi]

So it makes sense to add those methods also in the C++ interface. I will work on it tomorrow.

[GabrieleManduchi]

However, there is yes a slight difference, it appears that in python a Tree instantiation sets the default tree. In C++ not (and this is wanted)

[merlea]

One question then: is the setActiveTree action thread-safe ?

[GabrieleManduchi]

No, because the context is a static one. This is the reason why it should not be used unless strictly required. I am going to implement today the C++ version of Tree::tdiCompile(), Tree::tdiExecute() and Tree::tdiEvaluate() that will carry out the task in the context of the tree in a thread safe manner. In this way you don't need to call setActiveTree()

[merlea]

OK, very good. Thank you very much.

[GabrieleManduchi]

Just issued a PR with the Tree methods tdiData(), tdiEvaluate(), tdiCOmpile(), tdiExecute(). They are thread safe, PROVIDED you don't use anymore setActiveTree() (but now you have no reason for using it).

[GabrieleManduchi]

Hmmm......, the added tests unveiled a memory problem in Data.compile(). I need to fix it first before merging the new version (good to have discovered it). However I will be out of office next few days. Hopefully by the end of the next week the new version will be merged in alpha.

[tfredian]

According the the valgrind results of the fc26 build which failed, the memory problem is probably because the new test added to MdsTreeTest.cpp doesn't delete the tree instance.

[GabrieleManduchi]

I know, but this requires a change in the way Data.compile() was implemented. We did not realize this issue (it has been always there) because the corresponding test unit was missing. After adding the test units for tdiCompile() (and consequently for Data.compile()) we have discovered it.

[GabrieleManduchi]

The last version is a bit different from the previous one that used AutoData .However in that case the error was even worse because the Tree reference was deleted when going out of scope (i.e. when compile() returned), but a tree reference was maintained in the object hierarchy forming the compiled expression. This nasty situation has been uncovered only when I added a test unit for tdiCompile() (using Data.compile())

I am going on the mountains for a couple days now, but when I come back I will work out a solution

[GabrieleManduchi]

Finally the tests passed and the new version with Tree.tdixxx stuff has been merged