tenstorrent / tt-metal

:metal: TT-NN operator library, and TT-Metalium low level kernel programming model.
Apache License 2.0
430 stars 59 forks source link

Remove WH_ARCH_YAML from models and ops #11059

Open aliuTT opened 2 months ago

aliuTT commented 2 months ago

WH_ARCH_YAML is removed from runtime backend, CreateDevices and CreateDevice now asks for dispatch_core_type. So we will need model writers to update tests and conftest. Today, I added this logic so CI is clean:

def device(request, device_params):
    import tt_lib as ttl

    device_id = request.config.getoption("device_id")
    request.node.pci_ids = [ttl.device.GetPCIeDeviceID(device_id)]

    num_devices = ttl.device.GetNumPCIeDevices()
    assert device_id < num_devices, "CreateDevice not supported for non-mmio device"
    # TODO: 11059 move dispatch_core_type to device_params when all tests are updated to not use WH_ARCH_YAML env flag
    dispatch_core_type = ttl.device.CoreType.WORKER
    if os.environ["WH_ARCH_YAML"] == "wormhole_b0_80_arch_eth_dispatch.yaml":
        dispatch_core_type = ttl.device.CoreType.ETH
    device = ttl.device.CreateDevice(device_id=device_id, dispatch_core_type=dispatch_core_type, **device_params)

We can go with either a new fixture, maybe eth_dispatch_fixture or pass dispatch_core_type into device_params for the current device fixture. I have no preference either way. Once tests are updated to use either method, we can remove WH_ARCH_YAML from all test scripts.

Linking commit from main: https://github.com/tenstorrent/tt-metal/commit/509b2159ac3af3f6ad2301294a45df5bf1ce7d46

mtairum commented 1 month ago

Will this be back-propagated to all instances where the flag is used?

Or is it up to each model owner to check their model and update it?

aliuTT commented 1 month ago

I updated it for all the Metal CPP tests and a few fixtures, but I saw a lot of model/op specific tests that had this flag. I didn't feel confident modifying all of them, so leaving it to model owner to check their own tests.

aliuTT commented 3 weeks ago

Ping on this, any plans to get WH_ARCH_YAML removed from tests?

mtairum commented 2 weeks ago

@aliuTTYes, but low priority at the moment.