Add helper function for fixed points #71

doctorlai-msrc · 2025-04-01T18:26:02Z

Context

In jbpf codelets, the float point operations are not supported e.g. adding two floats would make the codelet unverifiable.

To overcome this limitation, we have added helper functions to convert float to fixed and vice versa.

Changes in this PR

This PR adds the following four helper functions in jbpf_helper_utils.h to convert the single/double to/from the fixedpt type.

static __always_inline double
fixed_to_double(fixedpt fixed);

static __always_inline float
fixed_to_float(fixedpt fixed);

static __always_inline fixedpt
float_to_fixed(float value);

static __always_inline fixedpt
double_to_fixed(double value);

This PR also adds tests to the fixedpt library aka. jbpf_helper_utils.h

Sample usages in codelet

The following is a verified codelet that shows the usage of the helper functions. In the codelet, two floats are defined, and converted to fixedpt for additions. Similarly two doubles are converted to fixedpt for the addition as well. It also demonstrates converting a float and a double from fixedpt to float and double respectively. All these values are passed back to C test for verification of correctness.

#include "jbpf_helper.h"
#include "jbpf_test_def.h"
#include "jbpf_helper_utils.h"

SEC("jbpf_generic")
uint64_t
jbpf_main(void* state)
{
    struct jbpf_generic_ctx* ctx = (struct jbpf_generic_ctx*)state;
    struct test_packet* data = (struct test_packet*)ctx->data;
    struct test_packet* data_end = (struct test_packet*)ctx->data_end;
    if (data + 1 > data_end) {
        return 1;
    }

    float a = 1.0;
    float b = 2.0;

    // 3.0 in fixedpt
    data->test_passed = float_to_fixed(a) + float_to_fixed(b);

    double aa = 3.0;
    double bb = 4.0;

    // 7.0 in fixedpt, so  3.0 + 7.0 = 10.0 which should be passed to the test_passed value
    data->test_passed += double_to_fixed(aa) + double_to_fixed(bb);

    data->test_passed_32 = fixed_to_float(fixedpt_rconst(12.3));
    data->test_passed_64 = fixed_to_double(fixedpt_rconst(45.6));

    return 0;
}

#70

bradunov · 2025-04-02T08:31:32Z

src/core/jbpf_fixpoint_utils.h

+ * Converts an unsigned integer to fixedpt, preserving accuracy.
+ */
+inline fixedpt
+fixedpt_from_uint(unsigned int value)


Why do we need this one? It seems to already exist in src/core/jbpf_helper_utils.h (under name fixedpt_toint)? If we need uint, shouldn't we add it as a macro in the headerfile, rather than a function?

bradunov · 2025-04-03T16:10:17Z

jbpf_tests/unit_tests/helper_functions/jbpf_fixpoint_test.c

+    {
+        float value = 1.0;
+        int32_t fixed_value = float_to_fixed(value);
+        float float_value = fixed_to_float(fixed_value);


Can you do a test to compare fixed_value with the expected one here too, here and elsewhere in this file? These are cheap tests that can be useful.

@bradunov I think I have those already: the tests (Test 14 to 17) in this same file

xfoukas · 2025-04-06T19:41:10Z

@doctorlai-msrc, can you please add a more detailed description about this PR? Specifically the assumptions and capabilities supported by the proposed conversions.

doctorlai-msrc · 2025-04-07T08:46:11Z

@doctorlai-msrc, can you please add a more detailed description about this PR? Specifically the assumptions and capabilities supported by the proposed conversions.

Added @xfoukas

xfoukas · 2025-04-07T10:40:36Z

jbpf_tests/unit_tests/helper_functions/jbpf_helper_utils_test.c

@@ -0,0 +1,327 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+/**


@doctorlai-msrc, we should have some tests, where you start with a float, which is converted to fixedpt, then we do some operations on it, like addition, subtraction, multiplication, etc, and then we convert back to float and check that we get the expected value.

@xfoukas sure, added.

xfoukas · 2025-04-09T09:57:28Z

jbpf_tests/test_files/codelets/helper_function_fixpoint/jbpf_helper_fixpoint.c

+        return 1;
+    }
+
+    float a = 1.0;


Why not use decimals here (e.g,, 1.25 and 3.124)?

xfoukas · 2025-04-09T09:59:11Z

jbpf_tests/test_files/codelets/helper_function_fixpoint/jbpf_helper_fixpoint.c

+    float b = 2.0;
+
+    // 3.0 in fixedpt
+    data->test_passed = float_to_fixed(a) + float_to_fixed(b);


We should be testing all the operations here, not just addition.

xfoukas · 2025-04-09T21:32:34Z

jbpf_tests/functional/helper_functions/jbpf_fixpoint_codelet_test.c

+        // the values are computed in the codelet
+        // Test case 1: convert two floats to fixedpt and add them, then convert back to float
+        // 1.23 + 2.34 = 3.57
+        assert(fabs(data.test_float_1 - 3.57) < 0.01);


Can you please explain why this needs to be tested with inequalities? Is the expected accuracy of the test not known a priori? The same question applies for all the other tests below.

because there is an precision loss performing float addition, especially, the floats are converted to fixedpt and then converted back.

But isn't the loss of precision known? Can it change depending on where the test runs? If not, why not check against specific values?

I don't understand. This is how we compare floats in the test. See here

xfoukas · 2025-04-09T21:33:55Z

jbpf_tests/functional/helper_functions/jbpf_fixpoint_codelet_test.c

+        // Test case 5: test float_to_fixed
+        assert(data.test_int_1 == fixedpt_rconst(1.2));
+        // Test case 6: test double_to_fixed
+        assert(data.test_int64_1 == fixedpt_rconst(2.345));


Shouldn't this be converted to float and then do a comparison between floats?

Test 3 to 6 are for each function, which are sufficient.

* Add helper function for fixed points * Addressing the comments * Revert * Continued work * Fix the algorithms and tests * Minor tweaks * Cleanup * Avoid clang optimise the code * Fix implementations * Test fixed_to_float and fixed_to_double * Fix type * Add 2 more tests * Cleanup * Clang format inconsistent fix * Add more tests in the codelet Co-authored-by: Zhihua Lai <[email protected]>

* Add helper function for fixed points (#71) * Add helper function for fixed points * Addressing the comments * Revert * Continued work * Fix the algorithms and tests * Minor tweaks * Cleanup * Avoid clang optimise the code * Fix implementations * Test fixed_to_float and fixed_to_double * Fix type * Add 2 more tests * Cleanup * Clang format inconsistent fix * Add more tests in the codelet * Increase MAX_NUM_HOOKS from 64 to 128 (#90) * Increase MAX_NUM_HOOKS from 64 to 96 * Increase MAX_NUM_HOOKS to 128 --------- Co-authored-by: Zhihua Lai <[email protected]>

* Add helper function for fixed points (#71) * Add helper function for fixed points * Addressing the comments * Revert * Continued work * Fix the algorithms and tests * Minor tweaks * Cleanup * Avoid clang optimise the code * Fix implementations * Test fixed_to_float and fixed_to_double * Fix type * Add 2 more tests * Cleanup * Clang format inconsistent fix * Add more tests in the codelet * Increase MAX_NUM_HOOKS from 64 to 128 (#90) * Increase MAX_NUM_HOOKS from 64 to 96 * Increase MAX_NUM_HOOKS to 128 --------- Co-authored-by: Zhihua Lai <[email protected]> Co-authored-by: matthewbalkwill <[email protected]>

* Add branch instructions in CONTRIBUTING.md and Update ADO pipeline triggers (#78) * Add Github Action Pipeline and Fixed INITIALIZE_SUBMODULES (#64) * Add Debug build configuration testing to CI pipeline (#57) --------- Co-authored-by: Zhihua Lai <[email protected]> Co-authored-by: Xenofon Foukas <[email protected]> * Add helper function for fixed points (#71) (#85) Co-authored-by: Zhihua Lai <[email protected]> * Fixes for tests to pass when cmake build type is Release (#86) This fixes issue #79. When the cmake build type is not defined it defaults to Release. Given that the tests rely on assert() to pass, the PR also undefs NDEBUG in the release build flags to prevent the optimizer from removing the assert() calls. * Improve Validation Error Messages in Linked Map (#82) * Matt/dev: MAX_NUM_HOOKS to 128 (#91) * Add helper function for fixed points (#71) * Increase MAX_NUM_HOOKS from 64 to 128 (#90) --------- Co-authored-by: Zhihua Lai <[email protected]> * Refactor run_lcm_subproc by removing memory allocation with new/delete. (#93) * Fix build/package error in ubuntu22.04 (#96) * Add native support for ARM builds (#59) --------- Co-authored-by: Zhihua Lai <[email protected]> Co-authored-by: Xenofon Foukas <[email protected]> Co-authored-by: Connor Settle <[email protected]> Co-authored-by: Zhihua Lai <[email protected]> * Bugfix: perf time measurement API with invalid times (#102) * Bump ubpf to the latest version (#103) * Matt/issue94 (#95) * Remove ado correctness + move to cloud agents (#100) * Sync dev with main (#105) * Add helper function for fixed points (#71) * Increase MAX_NUM_HOOKS from 64 to 128 (#90) --------- Co-authored-by: Zhihua Lai <[email protected]> Co-authored-by: matthewbalkwill <[email protected]> --------- Co-authored-by: Zhihua Lai <[email protected]> Co-authored-by: Xiaochuan Ye <[email protected]> Co-authored-by: matthewbalkwill <[email protected]> Co-authored-by: Anita Tenjarla <[email protected]> Co-authored-by: Zhihua Lai <[email protected]> Co-authored-by: Connor Settle <[email protected]>

* Adding a emulator * Add Python/ctypesgen dependencies for emulator * Add jbpf_agent_hooks.h and generate py * Add time helper functions * Test emulator * Add missing file * Add missing fields * Fix time event initialization * Refactor * Remove protobuf, update jbpf_stats_report.o * Add xran helper functions, and create random stream_id * Clang-format * Fix cppcheck issue * Refactor default values * Default num_codelet_descriptors to len(codelet_descriptors) * Enable ASAN in tests, ignore ODR, cleanup * Clangformat * Support custom helper functions and refactor * Fix clang-format * Move definitions above users include file * Remove XRAN * Add Custom init code * Add yaml_to_json, refactor * Fix path * Add debug option to emulator_utils.jbpf_handle_out_bufs * Match codelet_descriptor in yaml * Add missing define agent hooks * Revert * jbpf_handle_out_bufs should return how many messages that have been processed * Add some debug output when calling periodic_call and report_stats hook * Add a test to make sure the report_stats is actually called * Add comment * Clang-format -i * Add debug printf * Continued work * Add matching ck_epoch_end * Clang * Continued testing * Add more debugging * Clang * Debug * Debug print * Add more debugging message * More debugging * Add tests for report_stats and periodic_call, add debugging * Clang-format * Pipeline integration * simple_output2.o * Add missing files * Refactor & debug * COntinued work * Continued work * Revert and Cleanup * Revert * Fix * Ignore dynamic builds * Fix * Fix syntax * Fix syntax * Keep static mode only * Revert * Emulate time and add a test * Add test * Fix clang-format * Add missing file * Fix compilation under ubuntu20 * Init draft of adding both options * Fix * Pipeline * Move autogen wrappers * Fix cppcheck issue * Build both by default * Fix JBPF_SHARED_LIB * Fix build under ubuntu24.04 * Fix segfault at Static * Add emulator tests to github action, fix RELEASE * Typo * First round of addressing comments * Improve test description comments * Add some more doc * Add to ARM pipeline * Sync dev with main (#105) * Add helper function for fixed points (#71) * Add helper function for fixed points * Addressing the comments * Revert * Continued work * Fix the algorithms and tests * Minor tweaks * Cleanup * Avoid clang optimise the code * Fix implementations * Test fixed_to_float and fixed_to_double * Fix type * Add 2 more tests * Cleanup * Clang format inconsistent fix * Add more tests in the codelet * Increase MAX_NUM_HOOKS from 64 to 128 (#90) * Increase MAX_NUM_HOOKS from 64 to 96 * Increase MAX_NUM_HOOKS to 128 --------- Co-authored-by: Zhihua Lai <[email protected]> Co-authored-by: matthewbalkwill <[email protected]> * Refactor docker files * Add to github actions * Add System Information * Merge and fix * Fix comments * Readme * More doc * Update docs * Move emulator tests to the end * Add debug messages * Add more debug * Update codelet to provide more information * Comment * Fix print error * Revert "Update codelet to provide more information" This reverts commit 95dc9e4. * Fix segfault * Use smart ptr * Revert "Adding a emulator" * Revert "Enable Building Both libjbpf.a and libjbpf.so with cmake option -DJBPF_STATIC=Both" --------- Co-authored-by: Xenofon Foukas <[email protected]> Co-authored-by: matthewbalkwill <[email protected]>

Add helper function for fixed points

de350ad

doctorlai-msrc marked this pull request as ready for review April 1, 2025 18:39

doctorlai-msrc requested a review from bradunov April 1, 2025 18:39

doctorlai-msrc enabled auto-merge April 1, 2025 18:41

doctorlai-msrc self-assigned this Apr 1, 2025

doctorlai-msrc added the enhancement New feature or request label Apr 1, 2025

bradunov reviewed Apr 2, 2025

View reviewed changes

doctorlai-msrc added 6 commits April 2, 2025 15:28

Addressing the comments

2f3ce6b

Revert

fe756b1

Continued work

266ae59

Fix the algorithms and tests

60bb258

Minor tweaks

b1bb38a

Cleanup

8d60b6a

doctorlai-msrc requested a review from xfoukas April 3, 2025 15:43

bradunov reviewed Apr 3, 2025

View reviewed changes

doctorlai-msrc added 4 commits April 3, 2025 18:06

Avoid clang optimise the code

94da78c

Fix implementations

cee38fe

Test fixed_to_float and fixed_to_double

88e3916

Fix type

76a23af

xfoukas requested changes Apr 7, 2025

View reviewed changes

doctorlai-msrc added 3 commits April 7, 2025 13:12

Add 2 more tests

4c2a2c4

Cleanup

3db6cd5

Clang format inconsistent fix

a6f7c93

xfoukas requested changes Apr 9, 2025

View reviewed changes

Add more tests in the codelet

d04ee24

xfoukas reviewed Apr 9, 2025

View reviewed changes

xfoukas approved these changes Apr 15, 2025

View reviewed changes

doctorlai-msrc added this pull request to the merge queue Apr 15, 2025

Merged via the queue into main with commit 66c3f80 Apr 15, 2025
33 checks passed

doctorlai-msrc deleted the zhihua/fp branch April 15, 2025 14:38

doctorlai-msrc mentioned this pull request Apr 22, 2025

Tests crashed: cmake ../ -DCMAKE_BUILD_TYPE=Release is not the same as cmake ../ #79

Closed

		@@ -0,0 +1,327 @@
		// Copyright (c) Microsoft Corporation. All rights reserved.
		/**

Add helper function for fixed points #71

Add helper function for fixed points #71

Uh oh!

Conversation

doctorlai-msrc commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Changes in this PR

Sample usages in codelet

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bradunov Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xfoukas commented Apr 6, 2025

Uh oh!

doctorlai-msrc commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

doctorlai-msrc commented Apr 1, 2025 •

edited

Loading

bradunov Apr 3, 2025 •

edited

Loading