Submission checker modularization

[pgmpablo157321]

#1670
Testing command (outside the inference repo):

python -m inference.tools.submission.submission_checker.main --input inference_results_v5.1

[github-actions]

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

[nv-alicheng]

We can utilize the __init_subclass__ feature of Python to handle this registry-like functionality.

https://peps.python.org/pep-0487/#subclass-registration

I believe a better paradigm would be something like:

1. Implement a Checker subclass that inherits BaseChecker
2. Each Checker class will implement some number of methods that are prefixed with check_
3. BaseChecker's __call__ and execute() methods will check all attributes on the class and run (in sequence) all attributes that are callable and start with the string check_
4. If there is a dependency, we can implement an @BaseCheck.mark_dependency(<str>, ...) decorator where you can pass in a list of strings that are the function names, which need to be executed before the current check.

From what I could tell, all the implemented Check classes have an init method with the arguments log, path, config, submission_logs - The BaseCheck class should probably do the same and just store the values in self to be used by the subclasses later.

[nv-alicheng]

I'm wondering if it makes more sense to run every check here and return the success value of each, keyed by the checker's class? Something like:

{
    AccuracyCheck: True,
    ComplianceCheck: False,
    PerformanceCheck: True,
    ...
}

I can see this being clunky if many tests depend on each other, which means that check failures will cascade. In which case my question is should there be a system to determine the dependencies of each test? Something like MeasurementsCheck depends on DirectoryStructureCheck, etc.

[nv-alicheng]

Does it make more sense to have a ComplianceCheck class (which inherits BaseCheck), then have individual TEST0XCheck subclasses?

[nv-alicheng]

Can you add a formatter to the project like autopep8 or black?

[nv-alicheng]

The overloading of the term log here feels clunky and confusing. A few comments here:

1. It seems like bad practices to create a logger named 'main' and pass it around to each Checker. It would be better that each file has it's own logger (logging.getLogger(__file__)) so that if (for instance) ExampleCheck did log.info("Missing file ___"), the message in console would show that it originated in ExampleCheck rather than main.
2. If each file has its own logger, you no longer need to pass around log everywhere (makes it more concise)
3. If we are passing in logs, is there a point to also pass in logs.loader_data[key]? Can't that just be extracted by the Check's init method?
4. If this is simplified down to just xxxx_check = XXXXCheck(config, logs), then it can further be simplified down to

for logs in loader.load():
    for check_cls in [PerformanceCheck, ...]:
        check_cls(config, logs)()

[nv-alicheng]

Is this a TODO?

[nv-alicheng]

Why are these empty files here?

[nv-alicheng]

I mentioned this in the GH Issue, but if the giant model dict is already being stored in a Python file, it should probably be refactored into some hierarchy of dataclasses. Having a class based representation would also make doing the key -> property remapping you're doing here either easier or unnecessary.

Having it as dataclasses rather than a dict also makes the schema of the config more defined and easier to navigate.