Equality tests should feature .copy

[LonelyCat124]

Sergi and I think (and can't think of a counter example) that if you do .copy() on a node, the copy should be equal to the original.

We want to enforce this through a unit test.

[LonelyCat124]

@sergisiso would you be ok with a test like this?

import psyclone.psyir.nodes as nodes

def get_all_nodes():
    all_nodes = []
    for nod in dir(nodes):
        try:
            if isinstance(nodes.__dict__[x](), nodes.Node):
                all_nodes.append(nod)
        except:
            pass
    return all_nodes

@pytest.fixture(params=get_all_nodes())
def node(request):
    return request.param

def test_copy_equality(node):
    x = node()
    assert x.copy() == x

I assume this won't actually work though and certainly wouldn't cover every case inside a copy() call. Is there any other "smart" way we can do this or do you think a fully manual test suite is required?

[LonelyCat124]

This python script:

import psyclone.psyir.nodes as nodes

def get_all_nodes():
    all_nodes = []
    fails = 0
    for nod in dir(nodes):
        try:
            if isinstance(nodes.__dict__[nod](), nodes.Node):
                all_nodes.append(nodes.__dict__[nod])
        except Exception as e:
            pass
    for node in all_nodes:
        if not test_copy_equality(node):
            fails = fails + 1
    print(f"Finished for {len(all_nodes)} nodes and {fails} failed.")

def test_copy_equality(node):
    x = node()
    try:
        if(x.copy() != x):
            print(f"Fails for {type(x)}")
        return x.copy() == x
    except:
        print(f"Fails to copy for {type(x)}")
        return False

get_all_nodes()

gives this output:

DLHRT0063% python3 test.py
Fails for <class 'psyclone.psyir.nodes.acc_directives.ACCDataDirective'>
Fails for <class 'psyclone.psyir.nodes.acc_directives.ACCKernelsDirective'>
Fails for <class 'psyclone.psyir.nodes.acc_directives.ACCLoopDirective'>
Fails for <class 'psyclone.psyir.nodes.acc_directives.ACCParallelDirective'>
Fails for <class 'psyclone.psyir.nodes.acc_directives.ACCRegionDirective'>
Fails to copy for <class 'psyclone.psyir.nodes.loop.Loop'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPDoDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPLoopDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPMasterDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPParallelDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPParallelDoDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPRegionDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPSerialDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPSingleDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPTargetDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_task_directive.OMPTaskDirective'>
Fails for <class 'psyclone.psyir.nodes.omp_directives.OMPTaskloopDirective'>
Fails for <class 'psyclone.psyir.nodes.directive.RegionDirective'>
Fails for <class 'psyclone.psyir.nodes.schedule.Schedule'>
Fails for <class 'psyclone.psyir.nodes.scoping_node.ScopingNode'>

It works for anything that doesn't have a schedule as a child (i.e. a RegionDirective), doesn't have a symbol_table child (seemingly? I think this is why ScopingNode fails).

A loop created with Loop() also won't just copy straight up.

This raises a question regarding Schedule nodes - why does copy not result in equality?

[LonelyCat124]

Actually the answer here is just because its a ScopingNode, so then the issue is the symbol_table? But I'm not sure why it fails for RegionDirective (which has a Schedule child) but not IfBlock - I think the answer is just the former creates an empty schedule in the constructure, while IfBlock does not and is designed to be created using .create

[LonelyCat124]

It also gives us an easy answer to "when does .copy() not give equivalent nodes, and the answer appears to be if that Node contains a ScopingNode, which can never (?) be equal in a real tree? The test for ScopingNode equality is messy:

    symboltable = SymbolTable()
    symboltable2 = SymbolTable()
    # We need to manually set the same SymbolTable instance in both directives
    # for their equality to be True
    sched1 = Schedule()
    sched1._symbol_table = symboltable
    sched2 = Schedule()
    sched2._symbol_table = symboltable
    sched3 = Schedule(symbol_table=symboltable2)

    assert sched1 == sched2
    assert sched1 != sched3

I doubt any two schedule's in the PSyIR tree would actually have the same symboltable right? Maybe this equality check in scopingnode is bad. I think in any reasonable section of the tree, any two Schedules are already only equal if their children are, which requires any references contained within to point to the same symbol, which has to be is anyway, so is in the same scope.

Thoughts @sergisiso ?