Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Float Conv2D, Layernorm, Softmax, Reshape, Div, Relu #27

Closed
wants to merge 3 commits into from

Conversation

runwangdl
Copy link
Contributor

@runwangdl runwangdl commented Jan 22, 2025

Add Float Conv2D, LayerNorm, Div, Softmax, Reshape, Relu on Generic Platform

  • Float Conv2D, LayerNorm, Div, Reshape, Relu, Softmax basic classes
  • Add Float Conv2D, LayerNorm, Div, Reshape, Relu on Generic
  • Add Softmax on PulpOpen

Changed:

  • BasicIntergerDivBinding -> BasicDivBindings
  • IntergerDivLayer -> DivLayer
  • iSoftmaxLayer -> SoftmaxLayer
  • iLayernormLayer -> LayernormLayer
  • xxx binding -> bindings
  • IntegerDivChecker -> DivChecker
  • ilayernormchecker -> layernormchecker
  • isoftmaxchecker -> softmaxchecker
    (Note: nlevel, sign not used for float)

Added:

  • 6 float templates
  • relumapper, relulayer, relubinding, reluchecker
  • softmaxparser
  • layernormparser
  • divparser
  • reluparser

Deleted:

  • floataddchecker

Issue

  • Reshape __alias fail

PR Merge Checklist

  1. The PR is rebased on the latest devel commit and pointing to devel.
  2. Your PR reviewed and approved.
  3. All checks are passing.
  4. The CHANGELOG.md file has been updated.

@runwangdl runwangdl changed the title Float COnv2D, LayerNorm, Softmax, Reshape, Div, Relu [Draft] Float COnv2D, LayerNorm, Softmax, Reshape, Div, Relu Jan 22, 2025
@runwangdl runwangdl force-pushed the FloatOperator branch 3 times, most recently from aada6cd to d6fdb16 Compare January 23, 2025 00:25
@runwangdl runwangdl changed the title [Draft] Float COnv2D, LayerNorm, Softmax, Reshape, Div, Relu [Draft] Float Conv2D, Layernorm, Softmax, Reshape, Div, Relu Jan 23, 2025
@runwangdl runwangdl force-pushed the FloatOperator branch 2 times, most recently from dd6c3d3 to 92efdff Compare January 23, 2025 23:12
…n PulpOpen

- Float Conv2D, LayerNorm, Div, Reshape, Relu, Softmax basic classes
- Add Float Conv2D, LayerNorm, Div, Reshape, Relu on Generic
- Add Softmax on PulpOpen
@Victor-Jung Victor-Jung self-requested a review January 24, 2025 07:33
@runwangdl runwangdl marked this pull request as ready for review January 24, 2025 08:27
@runwangdl runwangdl changed the title [Draft] Float Conv2D, Layernorm, Softmax, Reshape, Div, Relu Float Conv2D, Layernorm, Softmax, Reshape, Div, Relu Jan 24, 2025
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some float test can not pass with diff 1e-5.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For which operator and with which shape?

Copy link
Member

@Victor-Jung Victor-Jung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First batch of comments is here. I didn't go through everything yet.

self.operatorRepresentation['data_in'] = data_in.name
self.operatorRepresentation['data_out'] = data_out.name
self.operatorRepresentation['size'] = np.prod(data_in.shape)
self.operatorRepresentation['lastDimLength'] = data_in.shape[-1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to know the last dimension length for RELU?

self.operatorRepresentation[outputs[idx]] = ctxt.lookup(outputNode.name).name

self.operatorRepresentation['size'] = np.prod(ctxt.lookup(self.operatorRepresentation['input1']).shape)
self.operatorRepresentation['lastDimLength'] = ctxt.lookup(self.operatorRepresentation['input1']).shape[-1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here, why would you need to know the length of the last dimension for a Div?

Comment on lines 36 to 42
def alignToContext(self, ctxt: NetworkContext,
operatorRepresentation: OperatorRepresentation) -> Tuple[NetworkContext, Dict]:

data_in = ctxt.lookup(operatorRepresentation['data_in'])
data_out = ctxt.lookup(operatorRepresentation['data_out'])

return ctxt, operatorRepresentation, []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method does not do anything, so why do you need it?

Comment on lines 29 to 32
class _reluTemplate(NodeTemplate):

def __init__(self, templateStr):
super().__init__(templateStr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this, just do `referenceTemplate = NodeTemplate(""""Your template""")

Comment on lines 31 to 42
class _FloatConvTemplate(NodeTemplate):

def __init__(self, templateStr):
super().__init__(templateStr)

def alignToContext(self, ctxt: NetworkContext,
operatorRepresentation: OperatorRepresentation) -> Tuple[NetworkContext, Dict, List[str]]:

data_in = ctxt.lookup(operatorRepresentation['data_in'])
data_out = ctxt.lookup(operatorRepresentation['data_out'])

return ctxt, operatorRepresentation, []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, this is class is useless

1. Delete useless classes in Templates
2. Fix relu dimension bug
self.operatorRepresentation['data_in'] = data_in.name
self.operatorRepresentation['data_out'] = data_out.name
self.operatorRepresentation['size'] = np.prod(data_in.shape)
self.operatorRepresentation['batch'] = data_in.shape[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to know the batch for the relu?

Comment on lines 31 to 40
void Relu_fp32_fp32(float32_t* input, float32_t* output, int32_t size, int32_t last_dim_length) {

int32_t batch_size = size / last_dim_length;

for (int b = 0; b < batch_size; b++) {
for (int i = 0; i < last_dim_length; i++) {
output[b * last_dim_length + i] = MAX(input[b * last_dim_length + i], 0.0f);
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relu is an element-wise function (unlike softmax or layernorm, which are row-wise). We don't need the last_dim_length here :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to align the function signature in the header ;)


referenceTemplate = NodeTemplate("""
// Relu (Name: ${nodeName}, Op: ${nodeOp})
SINGLE_CORE Relu_fp${data_in_type.referencedType.typeWidth}_fp${data_out_type.referencedType.typeWidth}(${data_in}, ${data_out}, ${size}, ${batch});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the batch here please, we don't need this for relu.

@@ -112,7 +113,8 @@
'RQIntegerDiv': RQIntegerDivLayer([RQIntegerDivMapper]),
'MatMul': MatMulLayer([MatMulMapper]),
'IntegerMean': ReduceMeanLayer([ReduceMeanMapper]),
'iSoftmax': iSoftmaxLayer([Softmax_int8_Mapper]),
'iSoftmax': SoftmaxLayer([Softmax_int8_Mapper]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: For class name we use PascalCase and for variable names we use camelCase. This is a very NIT comment but try to stick to the syle as much as you can please 😁

@runwangdl runwangdl closed this Jan 27, 2025
@runwangdl runwangdl deleted the FloatOperator branch February 6, 2025 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants