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

Add Decimation/Interpolation to node class. #141

Merged
merged 3 commits into from
Aug 1, 2023
Merged

Add Decimation/Interpolation to node class. #141

merged 3 commits into from
Aug 1, 2023

Conversation

drslebedev
Copy link
Contributor

@drslebedev drslebedev commented Jul 27, 2023

  • A special tag PerformDecimationInterpolation is required for node be enable decimation/interpolation.
  • Add ports_status to simplify port limits checks.
  • Add qa_node.cpp for unit tests of node class.
  • The stride functionality will be implemented in a separate PR: Implement stride for node class. #142

New

┌──────────────────────────────benchmark:───────────────────────────────┬──────┬─#N──┬─CTX-SW─┬───CPU cache misses────┬───CPU branch misses───┬─<CPU-I>─┬──min───┬──mean──┬─stddev─┬─median─┬──max───┬─total time─┬─ops/s─┐
│ merged src->sink work                                                 │ PASS │ 10  │     0  │ 2.09k / 3.86k = 54.2% │   390  / 212k =  0.2% │   10.4  │  14 us │  14 us │ 329 ns │  14 us │  15 us │     143 us │  716M │
│ merged src->copy->sink                                                │ PASS │ 10  │     0  │  643  / 1.38k = 46.7% │  203  / 11.3k =  1.8% │    1.1  │   4 us │   5 us │ 484 ns │   5 us │   5 us │      45 us │  2.3G │
│ merged src->copy->sink work                                           │ PASS │ 10  │     0  │   276  / 726  = 38.0% │   151  / 210k =  0.1% │   10.3  │  10 us │  12 us │ 648 ns │  11 us │  13 us │     116 us │  880M │
│ merged src->copy^10->sink                                             │ PASS │ 10  │     0  │  635  / 1.37k = 46.4% │   219  / 110k =  0.2% │    7.7  │  10 us │  11 us │ 357 ns │  11 us │  11 us │     110 us │  929M │
│ merged src(N=1024)->b1(N≤128)->b2(N=1024)->b3(N=32...128)->sink       │ PASS │ 10  │     0  │  720  / 1.46k = 49.3% │  204  / 10.8k =  1.9% │    1.1  │   4 us │   4 us │ 608 ns │   4 us │   5 us │      44 us │  2.4G │
│ merged src->mult(2.0)->divide(2.0)->add(-1)->sink - float             │ PASS │ 10  │     0  │  652  / 1.48k = 44.0% │   230  / 110k =  0.2% │   10.7  │  12 us │  12 us │ 569 ns │  12 us │  14 us │     124 us │  829M │
│ merged src->mult(2.0)->divide(2.0)->add(-1)->sink - int               │ PASS │ 10  │     0  │  628  / 1.39k = 45.1% │   220  / 110k =  0.2% │   11.7  │  17 us │  17 us │ 406 ns │  17 us │  18 us │     171 us │  598M │
│ merged src->(mult(2.0)->div(2.0)->add(-1))^10->sink - float           │ PASS │ 10  │     0  │  727  / 1.41k = 51.6% │  3.39k / 113k =  3.0% │   38.7  │ 108 us │ 132 us │  30 us │ 109 us │ 177 us │       1 ms │ 77.7M │
│ merged src->(mult(2.0)->div(2.0)->add(-1))^10->sink - int             │ PASS │ 10  │     0  │  691  / 1.41k = 48.9% │   219  / 117k =  0.2% │   48.8  │ 158 us │ 181 us │  39 us │ 159 us │ 257 us │       2 ms │ 56.4M │
│ runtime   src->sink overhead                                          │ PASS │ 10  │     0  │  13.5k / 181k =  7.4% │  2.67k / 441k =  0.6% │   21.9  │  13 us │  51 us │ 110 us │  14 us │ 382 us │     509 us │  201M │
│ runtime   src->copy->sink                                             │ PASS │ 10  │     0  │  25.8k / 353k =  7.3% │  4.09k / 746k =  0.5% │   34.8  │  14 us │  67 us │ 157 us │  15 us │ 537 us │     668 us │  153M │
│ runtime   src->copy^10->sink                                          │ PASS │ 10  │     0  │  114k / 1.91M =  6.0% │ 19.6k / 3.55M =  0.6% │    153  │  30 us │ 403 us │   1 ms │  37 us │   4 ms │       4 ms │ 25.4M │
│ runtime   src(N=1024)->b1(N≤128)->b2(N=1024)->b3(N=32...128)->sink    │ PASS │ 10  │     0  │  46.5k / 679k =  6.8% │ 7.15k / 1.46M =  0.5% │   67.0  │  21 us │ 165 us │ 413 us │  28 us │   1 ms │       2 ms │ 62.0M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - float             │ PASS │ 10  │     0  │  46.6k / 720k =  6.5% │ 8.25k / 1.39M =  0.6% │   62.7  │  20 us │ 156 us │ 395 us │  22 us │   1 ms │       2 ms │ 65.7M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - int               │ PASS │ 10  │     0  │  45.5k / 715k =  6.4% │ 8.23k / 1.40M =  0.6% │   64.1  │  19 us │ 157 us │ 403 us │  23 us │   1 ms │       2 ms │ 65.2M │
│ runtime   src->(mult(2.0)->div(2.0)->add(-1))^10->sink - float        │ PASS │ 10  │     0  │  315k / 5.61M =  5.6% │ 50.6k / 10.2M =  0.5% │    448  │ 121 us │   1 ms │   3 ms │ 131 us │  10 ms │      11 ms │  9.6M │
│ runtime   src->(mult(2.0)->div(2.0)->add(-1))^10->sink - int          │ PASS │ 10  │     0  │  292k / 5.61M =  5.2% │ 50.7k / 10.2M =  0.5% │    457  │ 129 us │   1 ms │   3 ms │ 136 us │   9 ms │      10 ms │  9.8M │
│ runtime   src->mult(2.0)->mult(0.5)->add(-1)->sink (SIMD)             │ PASS │ 10  │     0  │  46.2k / 706k =  6.6% │ 7.80k / 1.39M =  0.6% │   61.7  │  17 us │ 160 us │ 418 us │  21 us │   1 ms │       2 ms │ 64.0M │
│ runtime   src->(mult(2.0)->mult(0.5)->add(-1))^10->sink (SIMD)        │ PASS │ 10  │     0  │  270k / 5.43M =  5.0% │ 49.6k / 9.95M =  0.5% │    419  │  80 us │   1 ms │   3 ms │  87 us │  10 ms │      10 ms │  10.M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - float single      │ PASS │ 10  │    18  │ 1.20M / 6.32M = 18.9% │  215k / 10.7M =  2.0% │    549  │   2 ms │   2 ms │ 835 us │   2 ms │   4 ms │      20 ms │  5.1M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - int single        │ PASS │ 10  │    24  │ 1.16M / 6.25M = 18.5% │  195k / 10.6M =  1.8% │    543  │   2 ms │   2 ms │ 724 us │   2 ms │   4 ms │      19 ms │  5.5M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - float bulk        │ PASS │ 10  │     0  │  40.3k / 705k =  5.7% │ 8.02k / 1.42M =  0.6% │   63.0  │  20 us │ 150 us │ 368 us │  27 us │   1 ms │       1 ms │ 68.5M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - int bulk          │ PASS │ 10  │     0  │  37.9k / 704k =  5.4% │ 7.83k / 1.51M =  0.5% │   69.4  │  32 us │ 154 us │ 353 us │  36 us │   1 ms │       2 ms │ 66.6M │
└───────────────────────────────────────────────────────────────────────┴──────┴─────┴────────┴───────────────────────┴───────────────────────┴─────────┴────────┴────────┴────────┴────────┴────────┴────────────┴───────┘

main

┌──────────────────────────────benchmark:───────────────────────────────┬──────┬─#N──┬─CTX-SW─┬───CPU cache misses────┬───CPU branch misses───┬─<CPU-I>─┬──min───┬──mean──┬─stddev─┬─median─┬──max───┬─total time─┬─ops/s─┐
│ merged src->sink work                                                 │ PASS │ 10  │     0  │ 1.67k / 3.88k = 43.1% │   457  / 213k =  0.2% │   12.4  │  13 us │  13 us │ 334 ns │  14 us │  14 us │     134 us │  762M │
│ merged src->copy->sink                                                │ PASS │ 10  │     0  │  653  / 1.43k = 45.7% │  193  / 11.0k =  1.8% │    1.1  │   5 us │   6 us │ 404 ns │   6 us │   6 us │      55 us │  1.8G │
│ merged src->copy->sink work                                           │ PASS │ 10  │     0  │   259  / 759  = 34.1% │   139  / 209k =  0.1% │   12.2  │  11 us │  11 us │ 300 ns │  11 us │  12 us │     112 us │  913M │
│ merged src->copy^10->sink                                             │ PASS │ 10  │     0  │  674  / 1.46k = 46.1% │   237  / 110k =  0.2% │    7.7  │  10 us │  11 us │ 557 ns │  11 us │  12 us │     110 us │  933M │
│ merged src(N=1024)->b1(N≤128)->b2(N=1024)->b3(N=32...128)->sink       │ PASS │ 10  │     0  │  705  / 1.51k = 46.7% │  206  / 11.1k =  1.9% │    1.1  │   4 us │   5 us │ 530 ns │   5 us │   5 us │      46 us │  2.2G │
│ merged src->mult(2.0)->divide(2.0)->add(-1)->sink - float             │ PASS │ 10  │     0  │  655  / 1.47k = 44.6% │   213  / 110k =  0.2% │   10.7  │  11 us │  12 us │ 850 ns │  12 us │  14 us │     120 us │  852M │
│ merged src->mult(2.0)->divide(2.0)->add(-1)->sink - int               │ PASS │ 10  │     0  │  698  / 1.47k = 47.6% │   202  / 110k =  0.2% │   11.7  │  16 us │  17 us │ 390 ns │  17 us │  18 us │     170 us │  603M │
│ merged src->(mult(2.0)->div(2.0)->add(-1))^10->sink - float           │ PASS │ 10  │     0  │  745  / 1.49k = 50.0% │  3.39k / 114k =  3.0% │   38.8  │ 108 us │ 131 us │  30 us │ 109 us │ 177 us │       1 ms │ 78.3M │
│ merged src->(mult(2.0)->div(2.0)->add(-1))^10->sink - int             │ PASS │ 10  │     0  │  744  / 1.44k = 51.7% │   226  / 117k =  0.2% │   48.8  │ 159 us │ 171 us │  17 us │ 161 us │ 200 us │       2 ms │ 59.8M │
│ runtime   src->sink overhead                                          │ PASS │ 10  │     0  │  15.8k / 181k =  8.7% │  2.65k / 440k =  0.6% │   21.8  │  13 us │  42 us │  86 us │  14 us │ 301 us │     423 us │  242M │
│ runtime   src->copy->sink                                             │ PASS │ 10  │     0  │  28.4k / 353k =  8.0% │  4.29k / 746k =  0.6% │   34.7  │  13 us │  83 us │ 201 us │  16 us │ 687 us │     825 us │  124M │
│ runtime   src->copy^10->sink                                          │ PASS │ 10  │     0  │  118k / 1.93M =  6.1% │ 18.9k / 3.55M =  0.5% │    153  │  33 us │ 431 us │   1 ms │  39 us │   4 ms │       4 ms │ 23.8M │
│ runtime   src(N=1024)->b1(N≤128)->b2(N=1024)->b3(N=32...128)->sink    │ PASS │ 10  │     0  │  45.2k / 673k =  6.7% │ 7.35k / 1.45M =  0.5% │   66.3  │  21 us │ 173 us │ 431 us │  31 us │   1 ms │       2 ms │ 59.2M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - float             │ PASS │ 10  │     0  │  46.5k / 712k =  6.5% │ 7.87k / 1.38M =  0.6% │   62.0  │  19 us │ 158 us │ 402 us │  24 us │   1 ms │       2 ms │ 64.9M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - int               │ PASS │ 10  │     0  │  36.5k / 709k =  5.2% │ 7.84k / 1.38M =  0.6% │   63.0  │  21 us │ 210 us │ 548 us │  29 us │   2 ms │       2 ms │ 48.7M │
│ runtime   src->(mult(2.0)->div(2.0)->add(-1))^10->sink - float        │ PASS │ 10  │     0  │  305k / 5.61M =  5.4% │ 50.5k / 10.0M =  0.5% │    435  │ 107 us │   1 ms │   3 ms │ 117 us │   9 ms │      10 ms │  9.9M │
│ runtime   src->(mult(2.0)->div(2.0)->add(-1))^10->sink - int          │ PASS │ 10  │     0  │  300k / 5.57M =  5.4% │ 50.2k / 10.0M =  0.5% │    446  │ 120 us │   1 ms │   3 ms │ 125 us │   9 ms │      11 ms │  9.7M │
│ runtime   src->mult(2.0)->mult(0.5)->add(-1)->sink (SIMD)             │ PASS │ 10  │     0  │  42.6k / 706k =  6.0% │ 6.95k / 1.36M =  0.5% │   60.1  │  23 us │ 182 us │ 476 us │  24 us │   2 ms │       2 ms │ 56.2M │
│ runtime   src->(mult(2.0)->mult(0.5)->add(-1))^10->sink (SIMD)        │ PASS │ 10  │     0  │  284k / 5.42M =  5.2% │ 48.8k / 9.94M =  0.5% │    419  │  76 us │   1 ms │   3 ms │  80 us │  10 ms │      10 ms │  9.8M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - float single      │ PASS │ 10  │    25  │ 1.13M / 6.17M = 18.3% │  207k / 10.5M =  2.0% │    538  │   2 ms │   2 ms │ 798 us │   2 ms │   4 ms │      20 ms │  5.2M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - int single        │ PASS │ 10  │    26  │ 1.15M / 6.15M = 18.6% │  204k / 10.5M =  1.9% │    540  │   2 ms │   2 ms │ 775 us │   2 ms │   4 ms │      21 ms │  5.0M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - float bulk        │ PASS │ 10  │     0  │  39.5k / 742k =  5.3% │ 8.01k / 1.45M =  0.6% │   64.6  │  23 us │ 164 us │ 408 us │  29 us │   1 ms │       2 ms │ 62.5M │
│ runtime   src->mult(2.0)->div(2.0)->add(-1)->sink - int bulk          │ PASS │ 10  │     1  │  40.6k / 748k =  5.4% │ 8.49k / 1.63M =  0.5% │   77.1  │  52 us │ 198 us │ 428 us │  56 us │   1 ms │       2 ms │ 51.7M │
└───────────────────────────────────────────────────────────────────────┴──────┴─────┴────────┴───────────────────────┴───────────────────────┴─────────┴────────┴────────┴────────┴────────┴────────┴────────────┴───────┘

@drslebedev drslebedev linked an issue Jul 27, 2023 that may be closed by this pull request
@drslebedev drslebedev temporarily deployed to configure coverage July 27, 2023 15:34 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage July 27, 2023 15:34 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage July 27, 2023 15:34 — with GitHub Actions Inactive
@pr-explainer-bot
Copy link

Pull Request Review

Hey there! 👋 I've summarized the results of the previous tasks for you. Let's get started with the review!

Changes

  1. Added a new struct PerformDecimationInterpolation in annotated.hpp.
  2. Added a new operator= function in Annotated struct in annotated.hpp.
  3. Added a new operator std::string_view function in Annotated struct in annotated.hpp.
  4. Added a new unwrap_if_wrapped_t type trait in Annotated struct in annotated.hpp.
  5. Added a new typelist specialization in SupportedTypes struct in annotated.hpp.
  6. Added a new operator<< function in Annotated struct in annotated.hpp.
  7. Added a new samples_to_next_tag function in port.hpp.
  8. Added a new struct ports_status_t to track the status of input and output ports.
  9. Added a member variable ports_status of type ports_status_t to the node class.

Suggestions

  1. In annotated.hpp, line 62-63: Consider adding a brief description for the PerformDecimationInterpolation struct.
  2. In annotated.hpp, line 65-66: Consider adding a brief description for the BlockingIO struct.
  3. In annotated.hpp, line 133-134: Consider adding a brief description for the Annotated struct.
  4. In annotated.hpp, line 189-190: Consider adding a brief description for the unwrap_if_wrapped type trait.
  5. In port.hpp, line 605-606: Consider adding a brief description for the publish_tag function.
  6. In port.hpp, line 608-609: Consider adding a brief description for the samples_to_next_tag function.
  7. In the update_ports_status() function (lines 311-366), you can simplify the logic for updating ps.in_samples by using std::clamp instead of multiple if statements. Here's an example:
    ps.in_samples = std::clamp(ps.in_available, ps.in_min_samples, ps.in_max_samples);

Bugs

  1. In port.hpp, line 605-606: The publish_tag function may have a potential bug. The tag_offset parameter is not used in the function body.
  2. In the update_ports_status() function (lines 311-366), there is a potential bug in the calculation of ps.in_at_least_one_tag_available. The line ps.in_at_least_one_tag_available = ps.in_at_least_one_port_has_data | (port.tagReader().available() > 0); should use the logical OR operator (||) instead of the bitwise OR operator (|). Here's the corrected line:
    ps.in_at_least_one_tag_available = ps.in_at_least_one_port_has_data || (port.tagReader().available() > 0);

Improvements

  1. In annotated.hpp, line 133-134, the description() function can be refactored for better readability:
    static constexpr std::string_view description() noexcept {
        return description_;
    }
  2. In the update_ports_status() function (lines 311-366), you can improve readability by using range-based for loops instead of meta::tuple_for_each. Here's an example:
    for (auto& port : input_ports(&self())) {
        // Your code here
    }

Rating

Overall code rating: 8/10

Criteria:

  • Readability: 8/10
  • Performance: N/A
  • Security: N/A

That's it for the review! Let me know if you need any further assistance. Good luck with your pull request! 🚀

@drslebedev drslebedev temporarily deployed to configure coverage July 27, 2023 19:18 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage July 27, 2023 19:18 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage July 27, 2023 19:18 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 11:45 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 11:45 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 11:45 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 11:45 — with GitHub Actions Inactive
* A special tag PerformDecimationInterpolation is required for node be
enable decimation/interpolation.
* Add ports_status to simplify port limits checks.
* Add qa_node.cpp for unit tests of node class.
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@drslebedev drslebedev temporarily deployed to configure coverage August 1, 2023 12:12 — with GitHub Actions Inactive
@sonarcloud
Copy link

sonarcloud bot commented Aug 1, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 29 Code Smells

62.4% 62.4% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@wirew0rm
Copy link
Member

wirew0rm commented Aug 1, 2023

Force merging due to coverage requirements. We'll probably have to add some rules to make it possible to actually reach the coverage goal, there are e.g. some fmt constructs that generate branches that are not easily reachable.

@wirew0rm wirew0rm merged commit 691415a into main Aug 1, 2023
18 of 19 checks passed
@wirew0rm wirew0rm deleted the intdec branch August 1, 2023 13:21
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.

interpolator/decimator blocks
3 participants