Where in the code is this threshold of 10.39 or 20 being applied?

The current logic seems to just unconditionally apply exp and log. How does it avoid floating point overflow?

A threshold value of 10.39 or 20 was used in a previous revision, during which FP16 was supported in the Metal backend. In the current implementation, however, the logic unconditionally applies exp and log operations for FP32 data. Because the KataGo model normalizes inputs toward one, removing the threshold from the mish activation does not result in significant error. On the contrary, this change has led to a slight performance gain in the Metal backend, while maintaining very low numerical error.

The code comments regarding the threshold are retained above the mish function, as I may revisit FP16 support in the future to evaluate its impact on throughput and numerical accuracy in the Metal backend.

I have reintroduced the threshold in eda158f.

Are these unused now?

Thanks for finding these unused variables. I have remove them from the source code 86ab38f.

I think I prefer the previous version of the code, where there was:

  if(inputsUseNHWC != false) {
    throw StringError("Metal backend: inputsUseNHWC = false required, other configurations not supported");
  }

It means that we don't have the additional complexity of conversion here and it reduces the number of configurations to test. The KataGo engine above this is perfectly capable of specifying the input directly in NCHW format so that conversion is not needed, and there is no meaningful end-user functionality that is enabled by allowing KataGo's engine to supply a less efficient format and then to have the backend run an additional routine to convert it.

To make the tests still easy to run for the ones that try to set NHWC, the only additional thing that needs to be added I think is to tests/testcommon.cpp, in TestCommon::overrideForBackends, an additional case:

#elif defined(USE_METAL_BACKEND)
  if(inputsNHWC != false) {
    cout << "Backend is Metal, ignoring args and forcing inputsNHWC=false" << endl;
    inputsNHWC = false;
  }
  if(useNHWC != false) {
    cout << "Backend is Metal, ignoring args and forcing useNHWC=false" << endl;
    useNHWC = false;
  }

I'm cautious about adding a condition for the Metal backend in tests/testcommon.cpp. Although it would be logically valid, doing so introduces Metal-specific logic into a shared test file that I don’t primarily maintain. My main responsibility lies in metalbackend.{h,cpp,swift}, and I aim to keep changes outside of those files minimal. This approach helps keep the Metal backend modular for my maintenance.

Regarding test coverage, I can run your test cases to ensure both NCHW and NHWC formats are exercised. From my perspective, this should be sufficient and does not raise any concerns.

Additionally, since the Metal API natively supports the NHWC layout, the Metal backend has the potential to deliver better performance in future optimizations.

Okay cool. So let's leave this aspect alone for now.

There are actually an uncomfortable number of places in the current KataGo code outside of the backend code where inputsNHWC and possibly one or two other settings are hardcoded based on backend-specific knowledge as to the preferred orientation. So, I propose doing the following in a future version (not the coming release).

• I add a direct way in nninterface.h that all backends can report...
  • Their default/preferred setting for this and any other similar parameters
  • Which settings are supported, even if not preferred.
• Metal can then use this interface to report to the backend-agonistic part of KataGo that inputsNCHW is the preferred format (skipping the conversion of the format). Optionally, you could also then simplify the code by removing the conversion code, because the same interface can be used to inform KataGo that NHWC is not supported, in a backend-general way, and it will be a reliable part of the contract that then KataGo will never pass that format down to the backend.
  • If later you implement optimized support for NHWC as the faster format (which presumably will handle it directly rather than use the conversion logic), it will be easy to just change what the backend returns in that interface to report that NHWC is now supported and is the preferred format instead.
• In exactly the same way, OpenCL, TensorRT will also use this interface, and so everywhere we can remove the backend-specific hacks about input format.

Does that sound reasonable?

It sounds great!