ExtendedNeuralNetwork node #249
Conversation
tadeas0
requested review from
jkbmrz,
kkeroo,
klemen1999 and
tersekmatija
as code owners
aljazkonec1
left a comment
aljazkonec1
left a comment
There was a problem hiding this comment.
Took a quick look and left some comments. Mostly looks good, just a couple of minor fixes. Thanks!
Should we add tests aswell?
| """ | ||
|
|
||
| IMG_FRAME_TYPES = { | ||
| dai.Platform.RVC2: dai.ImgFrame.Type.BGR888p, |
There was a problem hiding this comment.
No need to add the frame type as RVC2 is not supported either way.
There was a problem hiding this comment.
General comment for entire PR: Use logger to log the stages of the class. Something like logger.info("Building TilingPipeline") and logger.info("Building Tiles patcher") etc etc.
There was a problem hiding this comment.
Agree, check the parsers implementation for reference.
depthai_nodes/node/tiles_patcher.py
Outdated
| assert isinstance( | ||
| nn_msg, self.SUPPORTED_MESSAGES | ||
| ), f"Message type {type(nn_msg)} is not supported." | ||
| if nn_msg.getTimestamp() != img.getTimestamp(): |
There was a problem hiding this comment.
could be > instead of != as the new nn_mseeage will have larger timestamp. If new message has lower timesptamp we would error out as that means we got an older message from previous frame.
| messages: List[GMessage], | ||
| ) -> GMessage: | ||
| if len(messages) == 0: | ||
| raise ValueError("Not enough messages to merge") |
There was a problem hiding this comment.
This is a valid state, it just means no detections / keypoints / ... are found in the frame. Should not error out, but just return a message with empty list of detections / keypoints / ...
There was a problem hiding this comment.
We can't infer the message type, thus cannot determine, what message type to return for empty lists.
There was a problem hiding this comment.
Maybe I'm missing something, but if no detections are in a frame the node will error out and crash the pipeline? Could we not just return an empty list [] in this case?
| return new_lines | ||
|
|
||
|
|
||
| def merte_predictions(predictions: List[Predictions]) -> Predictions: |
Unit tests would require mocking |
|
@tadeas0 FYI I am testing the new 1st and 2nd stage nodes in the focused vision PoC and have encountered an issue so we will likely not be merging either PR very quickly. I will let you know what the issue is. |
Purpose
Create
ExtendedNeuralNetworknode that wrapsParsingNeuralNetworkand adds the following capabilities:ImgDetectionsExtendedandImgDetectionsmessages)Specification
https://app.clickup.com/t/86c5mz8er
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
Manually tested on RVC4. Currently does not work on RVC2 due to missing transformation bindings in DAI
Scriptnode.Example usage
extended_neural_network_example.py