[erlang-patches] new float_to_list/2

Björn-Egil Dahlberg egil@REDACTED
Fri Jan 18 17:41:09 CET 2013


On 2013-01-18 17:25, Serge Aleynikov wrote:
> This is not intended.
>
> In both cases the issue is that the buffer used for formatting the
> numbers is fixed to 256 bytes.
Figured the buffer when I saw fbuf[256].
> 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?
I agree.

Documenting and enforcing a max limit should be the appropriate thing to do.
Also, test the error cases.

// Björn-Egil
>
> 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