[erlang-patches] new float_to_list/2

Serge Aleynikov serge@REDACTED
Fri Jan 18 17:25:26 CET 2013 

This is not intended.

In both cases the issue is that the buffer used for formatting the
numbers is fixed to 256 bytes.

So essentially in case of {scientific, Decimals} option the C call looks
like this:

    erts_snprintf(buffer, buffer_size, "%.*e", decimals, n)

In case of {decimals, Decimals} option, when Decimals exceeds the buffer
size, the call defaults to:

    erts_snprintf(buffer, buffer_size-1, "%.*f", Decimals, n);

The question is what should be the function's behavior when the Decimals
value is (too) large?  Throw badarg?  Reduce it to a set maximum?  I
think that documenting a max limit and throwing badarg would be the
right approach.

Thoughts?

On 1/18/2013 9:59 AM, Björn-Egil Dahlberg wrote:
> In review .. again =)
> 
> I think we have covered default behaviours.
> 
> Is the following intended behaviour? =)
> 
> 1> erlang:float_to_list(3/7, [{decimals, 314},
> compact]).                    
> [48,46,52,50,56,53,55,49,52,50,56,53,55,49,52,50,56,53,52,
>  55,54,51,56,48,55,56,48,52,51|...]
> 
> 2> erlang:float_to_list(3/7, [{scientific, 314}, compact]).              
> "4.2857142857142854763807804374664556235074996948242187500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
> 
> ?
> 
> // Björn-Egil
> 
> On 2013-01-14 10:10, Fredrik wrote:
>> Hello,
>> This testcase is failing:
>> Suite: bif_SUITE
>> Testcase: specs
>> Reason:
>> The following BIFs don't have specs:
>>
>> erlang:float_to_list/2
>> Give me notice when this is done,
>> BR Fredrik Gustafsson
>> Erlang OTP Team
>> On 01/11/2013 04:55 PM, Serge Aleynikov wrote:
>>> Sorry the test case line was the "last moment" add on.  It is fixed now
>>> in my branch below.
>>>
>>> On 1/11/2013 10:44 AM, Fredrik wrote:
>>>> Hey,
>>>> Your patch does not build:
>>>> "
>>>>
>>>> emulator_test ../emulator_test/num_bif_SUITE.erl:130: illegal pattern
>>>> "
>>>>
>>>>
>>>> Please have a look at it, and give me notice when you are done.
>>>>
>>>> BR Fredrik Gustafsson
>>>> Erlang OTP Team
>>>> On 01/11/2013 03:11 PM, Serge Aleynikov wrote:
>>>>> Ok. Here's the patch rebased to master:
>>>>>
>>>>> git fetch https://github.com/saleyn/otp/tree/float_to_list_2
>>>>>
>>>>> https://github.com/saleyn/otp/compare/master...float_to_list_2
>>>>> https://github.com/saleyn/otp/compare/master...float_to_list_2.patch
>>>>>
>>>>> Regards,
>>>>>
>>>>> Serge
>>>>>
>>>>> On 1/11/2013 8:53 AM, Björn-Egil Dahlberg wrote:
>>>>>> On 2013-01-11 14:24, Serge Aleynikov wrote:
>>>>>>> While looking at this I see that the code in folder
>>>>>>> "erts/emulator/sys/vxworks" is present in maint but missing in the
>>>>>>> master branch.  Is this intentional?  If so, should I remove the
>>>>>>> part of
>>>>>>> the patch designed for vxworks?
>>>>>> Yes. Any and all vxworks support has been removed in R16 from OTP except
>>>>>> for epmd, erl_interface and ic.
>>>>>>
>>>>>> // Björn-Egil
>>>>>>> On 1/11/2013 3:52 AM, Fredrik wrote:
>>>>>>>> Hey,
>>>>>>>> Could you please rebase this on current 'master' branch?
>>>>>>>>
>>>>>>>> BR Fredrik Gustafsson
>>>>>>>> Erlang OTP Team
>>>>>>>> On 01/11/2013 04:06 AM, Serge Aleynikov wrote:
>>>>>>>>> I implemented Lukas's recommendations that were presenting an
>>>>>>>>> acceptance
>>>>>>>>> issue of the new function, so the current version of the patch does:
>>>>>>>>>
>>>>>>>>> 1. float_to_list/1 is backwards compatible.
>>>>>>>>> 2. float_to_list(X,[]) gives the same result as float_to_list(X).
>>>>>>>>> 3. float_to_list(X,[{decimals,N}]) uses the new fast implementation
>>>>>>>>> (with the optional compact option).
>>>>>>>>> 4. float_to_list(X,[{scientific,M}]) gives the same result as
>>>>>>>>> float_to_list(X) with the ability to control the number of decimals.
>>>>>>>>>
>>>>>>>>> The only item from Lucas's list that I left unchanged was the
>>>>>>>>> modification to erts/lib_src/common/erl_printf_format.c to take
>>>>>>>>> advantage of the speed improvement of the new implementation.  I am
>>>>>>>>> including a patch in this email that implements this logic, but I
>>>>>>>>> decided to leave the integration task to the OTP team since
>>>>>>>>> erl_printf_format.c is actually compiled into a liberts_internal.a
>>>>>>>>> library and I didn't want to introduce a dependency of it on other
>>>>>>>>> code
>>>>>>>>> - this should be decided by maintainers.
>>>>>>>>>
>>>>>>>>> The test cases of float_to_list/{1,2} have been updated.
>>>>>>>>>
>>>>>>>>> git fetch https://github.com/saleyn/otp/tree/float_to_list_2
>>>>>>>>>
>>>>>>>>> https://github.com/saleyn/otp/compare/maint...float_to_list_2
>>>>>>>>> https://github.com/saleyn/otp/compare/maint...float_to_list_2.patch
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>>
>>>>>>>>> Serge
>>>>>>>>>
>>>>>>>>> On 8/23/2012 5:10 AM, Lukas Larsson wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> I'll put it in the backlog and we'll see if it gets prioritized for
>>>>>>>>>> R16B.
>>>>>>>>>>
>>>>>>>>>> As always if you (or someone else) wants make sure it gets in, the
>>>>>>>>>> best
>>>>>>>>>> way to ensure that is to send an updated patch.
>>>>>>>>>>
>>>>>>>>>> Lukas
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 22/08/12 21:17, Serge Aleynikov wrote:
>>>>>>>>>>> I am certainly very happy to hear that you finally agreed to
>>>>>>>>>>> include
>>>>>>>>>>> this in the distribution.  The changes proposed below seem
>>>>>>>>>>> reasonable
>>>>>>>>>>> and simple.  Will the OTP team be able to modify my patch to
>>>>>>>>>>> implement
>>>>>>>>>>> them?
>>>>>>>>>>>
>>>>>>>>>>> On 8/22/2012 12:01 PM, Lukas Larsson wrote:
>>>>>>>>>>>> Hello Serge!
>>>>>>>>>>>>
>>>>>>>>>>>> I think we have finally agreed how we want this functionality to
>>>>>>>>>>>> work.
>>>>>>>>>>>>
>>>>>>>>>>>> float_to_list/1 should be left as it is now for backwards
>>>>>>>>>>>> compatibility.
>>>>>>>>>>>> float_to_list(1.0,[]) should give the same as float_to_list(1.0).
>>>>>>>>>>>> float_to_list(1.0,[{decimals,X}]) should use your faster
>>>>>>>>>>>> implementation
>>>>>>>>>>>> (with the optional compact option).
>>>>>>>>>>>> float_to_list(1.0,[{scientific,Y}]) should give the same as
>>>>>>>>>>>> float_to_list(1.0) if Y is 20.
>>>>>>>>>>>>
>>>>>>>>>>>> We would also like the string rendering part of
>>>>>>>>>>>> sys_double_to_chars_fast
>>>>>>>>>>>> to be put into
>>>>>>>>>>>> erts/lib_src/common/erl_printf_format.c:fmt_double. This
>>>>>>>>>>>> way other parts of the vm which print floats will benefit from
>>>>>>>>>>>> your
>>>>>>>>>>>> changes!
>>>>>>>>>>>>
>>>>>>>>>>>> I hope our indecisiveness have not caused you to shy away from
>>>>>>>>>>>> taking
>>>>>>>>>>>> this feature into Erlang/OTP. If you have any further questions or
>>>>>>>>>>>> ponderings just let me know.
>>>>>>>>>>>>
>>>>>>>>>>>> Lukas
>>>>>>>>>>>>
>>>>>>>>>>>> On 18/07/12 20:47, Serge Aleynikov wrote:
>>>>>>>>>>>>> On 7/18/2012 5:18 AM, Lukas Larsson wrote:
>>>>>>>>>>>>>> However, I would also like the fast functionality to be used by
>>>>>>>>>>>>>> float_to_list_1 as well, is possible to do this and stay
>>>>>>>>>>>>>> backwards
>>>>>>>>>>>>>> compatible? Hopefully you just have to shift the comma and add
>>>>>>>>>>>>>> e+XX at
>>>>>>>>>>>>>> the end of the optimized case and call sys_double_to_chars
>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>> unoptimized.
>>>>>>>>>>>>> See my comments below regarding 'scientific' option.  Since
>>>>>>>>>>>>> float_to_list_2 is a new function I would think that you are
>>>>>>>>>>>>> questioning
>>>>>>>>>>>>> the issue of backward compatibility only in terms of converting
>>>>>>>>>>>>> float_to_list_1 to use float_to_list_2 implementation. 
>>>>>>>>>>>>> However, I
>>>>>>>>>>>>> think
>>>>>>>>>>>>> that this will have adverse performance tax on float_to_list_2,
>>>>>>>>>>>>> which
>>>>>>>>>>>>> will diminish the benefit.  It's been a while however, since I
>>>>>>>>>>>>> wrote
>>>>>>>>>>>>> that patch, perhaps there's a way to retrofit scientific notation
>>>>>>>>>>>>> without a performance penalty.
>>>>>>>>>>>>>
>>>>>>>>>>>>> It's more than simple shifting of the comma, since there's also
>>>>>>>>>>>>> rounding
>>>>>>>>>>>>> involved.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This case is easy:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 4>  float_to_list(1.01234).
>>>>>>>>>>>>> "1.01234000000000001762e+00"
>>>>>>>>>>>>> 5>  float_to_list(1.01234, [{decimals, 20}]).
>>>>>>>>>>>>> "1.01234000000000001762"
>>>>>>>>>>>>>
>>>>>>>>>>>>> This case is a bit more complex (illustration of rounding
>>>>>>>>>>>>> impact):
>>>>>>>>>>>>>
>>>>>>>>>>>>> 7>  float_to_list(10123412345.0123451234).
>>>>>>>>>>>>> "1.01234123450123443604e+10"
>>>>>>>>>>>>> 8>  float_to_list(10123412345.0123451234, [{decimals, 20}]).
>>>>>>>>>>>>> "10123412345.01234436035156250000"
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Also float_to_list(1.0) should return the same thing as
>>>>>>>>>>>>>> float_to_list(1.0,[]), otherwise the API will be inconsistent
>>>>>>>>>>>>>> with how
>>>>>>>>>>>>>> other such APIs work.
>>>>>>>>>>>>> Actually if you trace this subject back there was another
>>>>>>>>>>>>> request that
>>>>>>>>>>>>> the default number of decimals is chosen to be consistent with
>>>>>>>>>>>>> what
>>>>>>>>>>>>> printf() does, so I changed the implementation to accommodate
>>>>>>>>>>>>> that
>>>>>>>>>>>>> request:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Eshell V5.9  (abort with ^G)
>>>>>>>>>>>>> 1>  float_to_list(1.0, []).
>>>>>>>>>>>>> "1.000000"
>>>>>>>>>>>>>
>>>>>>>>>>>>> Which is different from the default of float_to_list/1:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2>  float_to_list(1.0).
>>>>>>>>>>>>> "1.00000000000000000000e+00"
>>>>>>>>>>>>>
>>>>>>>>>>>>> Maybe one could introduce a 'scientific' option to
>>>>>>>>>>>>> float_to_list/2, to
>>>>>>>>>>>>> use the float_to_list/1 implementation?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Serge
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 24/05/12 16:02, Serge Aleynikov wrote:
>>>>>>>>>>>>>>> Henrik,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Fetch:    git fetch
>>>>>>>>>>>>>>> https://github.com/saleyn/otp/tree/float_to_list_2
>>>>>>>>>>>>>>> Diff:    https://github.com/saleyn/otp/compare/float_to_list_2
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I added the definition for the new BIF to make the type checker
>>>>>>>>>>>>>>> happy:
>>>>>>>>>>>>>>> https://github.com/saleyn/otp/commit/f9ddbeda5426ca83cda03c06a9860220ea4a22c7
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Once you do the "otp_build tests", how do you execute all tests
>>>>>>>>>>>>>>> suites
>>>>>>>>>>>>>>> in $ERL_TOP or if possible only tests in a given SUITE?  I
>>>>>>>>>>>>>>> tried the
>>>>>>>>>>>>>>> following but all tests fail:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> [otp/erts/emulator/test]$ ../../../bin/erl -noshell -s
>>>>>>>>>>>>>>> test_server_ctrl
>>>>>>>>>>>>>>> run_test DIR "." -s erlang halt
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I did however run individual tests in bif_SUIT:types to make
>>>>>>>>>>>>>>> sure my
>>>>>>>>>>>>>>> patch didn't break anything.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Serge
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 5/24/2012 5:17 AM, Henrik Nord wrote:
>>>>>>>>>>>>>>>> Hi again.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This test is not passing: emulator/bif_SUIT:types
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> No type information:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> [{erlang,float_to_list,2}]
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 04/21/2012 07:19 AM, Serge Aleynikov wrote:
>>>>>>>>>>>>>>>>> git fetch https://github.com/saleyn/otp/tree/float_to_list_2
>>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>>> erlang-patches mailing list
>>>>>>>>>>>>>>> erlang-patches@REDACTED
>>>>>>>>>>>>>>> http://erlang.org/mailman/listinfo/erlang-patches
>>>>>>>>>>>>>>>
>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> erlang-patches mailing list
>>>>>>>>>>>>> erlang-patches@REDACTED
>>>>>>>>>>>>> http://erlang.org/mailman/listinfo/erlang-patches
>>>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> erlang-patches mailing list
>>>>>>>>>>> erlang-patches@REDACTED
>>>>>>>>>>> http://erlang.org/mailman/listinfo/erlang-patches
>>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> erlang-patches mailing list
>>>>>>>>>> erlang-patches@REDACTED
>>>>>>>>>> http://erlang.org/mailman/listinfo/erlang-patches
>>
>>
>>
>> _______________________________________________
>> erlang-patches mailing list
>> erlang-patches@REDACTED
>> http://erlang.org/mailman/listinfo/erlang-patches
> 
> 
> 
> _______________________________________________
> erlang-patches mailing list
> erlang-patches@REDACTED
> http://erlang.org/mailman/listinfo/erlang-patches
>

More information about the erlang-patches mailing list