MDMM tests and hardware-aware metric functions#37
MDMM tests and hardware-aware metric functions#37ArghyaRanjanDas wants to merge 2 commits intocern-nextgen:mainfrom
Conversation
Tests cover metric functions, constraint functions, training phases, mask correctness, penalty numerical verification, constraint type variations, L0 mode variations, and edge cases.
|
We can't merge directly to main branch unless it was merged to a dev one @ArghyaRanjanDas |
|
After testing MDMM tests and training within model training loop, I detected an issue with PydanticSerializationUnexpectedValue, especially:
|
nastiapetrovych
left a comment
nastiapetrovych
left a comment
There was a problem hiding this comment.
Hi @ArghyaRanjanDas ,
There are several several modifications that should be corrected before merging to the dev branch.
|
|
||
| def __init__(self, rf=1, precision=16, target_resource='DSP', | ||
| bram_width=36, epsilon=1e-3): | ||
| assert target_resource in ['DSP', 'BRAM'], "target_resource must be 'DSP' or 'BRAM'." |
There was a problem hiding this comment.
Seems to be overkill to assert additional parameters asserts of FPGAAwareSparsityMetric, taking into account that Pydantic classes are used for this ( to validate after config upload ).
I would recommend to set numerical values in the model with additional parameters ge for > , le for <. Also, the target_resource value must be already validated, as you are using Literal.
| # src: | ||
| # PyTorch: (out, in, kH, kW): OIHW | ||
| # Keras : (kH, kW, in, out): HWIO | ||
| w_permuted = convert_conv_layout(w, src="OIHW", dst="OIHW") |
There was a problem hiding this comment.
In this command line, src="OIHW" and dst="OIHW", so basically there is no permutation conducted. Do we need this?
|
|
||
| def _get_unique_patterns_with_counts(all_patterns): | ||
| """Returns the unique patterns and their counts.""" | ||
| np_patterns = ops.convert_to_numpy(all_patterns) |
There was a problem hiding this comment.
Can be problematic if we want to track a computational graph
| P_exp = ops.expand_dims(P, 0) | ||
|
|
||
|
|
||
| if distance_metric == 'hamming': |
There was a problem hiding this comment.
I would add distance_metic dict in constants and reuse it here in the code
| prepared_weights = self._prepare_weights(weight) | ||
| dsp_groups = ops.reshape(prepared_weights, (prepared_weights.shape[0], -1, self.rf)) | ||
|
|
||
| if self.target_resource == 'DSP': |
There was a problem hiding this comment.
the same here, target_resource dict in constants and reuse in the code, strings aren't prefered
|
|
||
| def __init__(self, num_patterns_to_keep=16, beta=0.75, | ||
| epsilon=1e-5, distance_metric='valued_hamming'): | ||
| assert num_patterns_to_keep > 0, "num_patterns_to_keep must be > 0" |
There was a problem hiding this comment.
back to previous comments, all validation should be done in Pydantic class
| # PACAPatternMetric-specific | ||
| num_patterns_to_keep: 16 | ||
| beta: 0.85 | ||
| distance_metric: "cosine" # "hamming", "valued_hamming", or "cosine" |
There was a problem hiding this comment.
these details should be also added on the documentation page
| scale_mode: Literal["mean", "sum"] = Field(default="mean") | ||
| constraint_lr: float = Field(default=1.0e-3) | ||
| # FPGAAwareSparsityMetric (only used when metric_type == "FPGAAwareSparsity") | ||
| precision: Optional[int] = Field(default=None) |
There was a problem hiding this comment.
I would add here metric_type and move these parameters to corresponding metric classes: FPGAAwareSparsityMetric and PACAPatternMetric
| import keras | ||
| from keras import ops | ||
|
|
||
| _AX = {"H": 0, # kernel height (kH) |
There was a problem hiding this comment.
can be moved to constants?
| return ops.sum(ops.cast(zero_bram_groups, dsp_groups.dtype)) / num_bram_groups | ||
|
|
||
|
|
||
| class PACAPatternMetric: |
There was a problem hiding this comment.
i would say so make these metric classes pydantic
2 participants
Adds tests for the existing MDMM pruning method and integrates the hardware-aware metric functions (
FPGAAwareSparsityMetricandPACAPatternMetric) that were lost during intermediate branch merging.Also fixes a
float32/int32dtype bug inget_layer_sparsityon the TensorFlow backend.Supported backends
KERAS_BACKEND=tensorflowKERAS_BACKEND=torchTest files
pytest tests/test_mdmm.py tests/test_mdmm_metrics.py