View mode: basic / threaded / horizontal-split · Log in · Help
July 30, 2012
Re: sorting failed error
2012/7/31 Timon Gehr <timon.gehr@gmx.ch>:
> I realize that the code is just for fun, but I have some comments:
>
> - don't .dup stuff just in order to index directly. it never has
> any effect other than performance degradation. (this could be a
> compiler warning)
>
> - crossOver and mutate mutate their parameters in-place. I assume this
> is by mistake. reproduce effectively inserts every element a couple
> times because of this -- this is why the bug manifests reliably.
> (const, immutable annotations?)
>
> - make use of $
>
> int from=uniform(0,victim.dna.length);
> int to=uniform(0,victim.dna.length);
> swap(victim.dna[from],victim.dna[to]);
>
> =>
>
> swap(victim.dna[uniform(0,$)],
>      victim.dna[uniform(0,$)]);
>
> - sort is in-place
>
> workers=array(sort!(fitnessCompare)(workers));
>
> =>
>
> sort!fitnessCompare(workers)
>
> - mark local functions static if they do not need to access the
> enclosing context.
>
> - use size_t for array iteration variables (if necessary)
>
>
> for(int x=0;x<cities.length-1;x++)
>     travelled+=distance(cities[x],cities[x+1]);
>
> =>
>
> for(size_t x=1;x<cities.length;x++)
>     travelled+=distance(cities[x-1],cities[x]);
>
> I also removed the subtraction from the array length. This would be
> correct in this case because cities.length>=2 is guaranteed, but it is
> an error prone pattern.
>
> - another way to do it is to drop the loop completely and to use
> ranges instead:
>
> return zip(cities,cities.drop(1))
>       .map!(a=>distance(a.expand))
>       .reduce!"a+b";
>
>
> The benefit is that this is now obviously correct.
>
>
> You might also want to consider not building up the cities array
> and computing the terms at the boundary explicitly instead:
>
> return distance(city(0,0), victim.dna[0]) +
>        distance(victim.dna[$-1], city(0,0)) +
>        zip(victim.dna, victim.dna.drop(1))
>       .map!(a=>distance(a.expand))
>       .reduce!"a+b";
>
> The goal is to craft the shortest code possible that is still
> efficient and easy to follow.
>
> - type deduction?
>
>
> further comments whose application does not lead to immediate benefit:
>
> - in contracts are better specified in their dedicated section to push
> the requirements onto the caller.
>
> - consider for(;;) as a means for indefinite looping.
>
> - the convention is upper case for type names

Thank you very much for this criticism, I'm relatively new to
programming and these mistakes/points are the kind off things you
can't learn from reading a book or two.

I have one little question about one of the last points though
why use for(;;)?
As far as I can tell this simply reduces readability from while(true).
Is there any reason for this?
July 30, 2012
Re: sorting failed error
On 07/31/2012 12:30 AM, maarten van damme wrote:
> 2012/7/31 Timon Gehr<timon.gehr@gmx.ch>:
>> ...
>> further comments whose application does not lead to immediate benefit:
>>
>> - in contracts are better specified in their dedicated section to push
>> the requirements onto the caller.
>>
>> - consider for(;;) as a means for indefinite looping.
>>
>> - the convention is upper case for type names
>
> Thank you very much for this criticism, I'm relatively new to
> programming and these mistakes/points are the kind off things you
> can't learn from reading a book or two.
>

I'm glad to help.

> I have one little question about one of the last points though
> why use for(;;)?

Well, as stated, there is no benefit.

> As far as I can tell this simply reduces readability from while(true).

That is entirely a matter of preference.

> Is there any reason for this?

I like it more because it says "loop".
while(true) says: "loop as long as 'true' still holds", which is silly.
July 30, 2012
Re: sorting failed error
On 7/31/12, Timon Gehr <timon.gehr@gmx.ch> wrote:
> I like it more because it says "loop".

I'd love to have "loop { }". But keyword bloat yada yada. :)
July 31, 2012
Re: sorting failed error
Jonathan M Davis:

> I'd actually argue that structs without opCmp shouldn't have it 
> implicitly defined. It's just begging for bugs otherwise.

For reference, this is the issue we are talking about:
http://d.puremagic.com/issues/show_bug.cgi?id=3789

The current situation is totally not acceptable. Even doing what 
I think you are suggesting, that is generating a compile-time 
error, is better than the current situation, that leads to silent 
bugs.

Bye,
bearophile
July 31, 2012
Re: sorting failed error
On Monday, 30 July 2012 at 22:58:28 UTC, Timon Gehr wrote:
> On 07/31/2012 12:30 AM, maarten van damme wrote:
>> 2012/7/31 Timon Gehr<timon.gehr@gmx.ch>:
>>> ...
>>> further comments whose application does not lead to immediate 
>>> benefit:
>>>
>>> - in contracts are better specified in their dedicated 
>>> section to push
>>> the requirements onto the caller.
>>>
>>> - consider for(;;) as a means for indefinite looping.
>>>
>>> - the convention is upper case for type names
>>
>> Thank you very much for this criticism, I'm relatively new to
>> programming and these mistakes/points are the kind off things 
>> you
>> can't learn from reading a book or two.
>>
>
> I'm glad to help.
>
>> I have one little question about one of the last points though
>> why use for(;;)?
>
> Well, as stated, there is no benefit.
>
>> As far as I can tell this simply reduces readability from 
>> while(true).
>
> That is entirely a matter of preference.
>
>> Is there any reason for this?
>
> I like it more because it says "loop".
> while(true) says: "loop as long as 'true' still holds", which 
> is silly.

I've usually see the "for(;;)" syntax to loop infinity: This 
reads as: Just keep looping, with no condition.
July 31, 2012
Re: sorting failed error
On Tuesday, 31 July 2012 at 00:21:59 UTC, bearophile wrote:
> Jonathan M Davis:
>
>> I'd actually argue that structs without opCmp shouldn't have 
>> it implicitly defined. It's just begging for bugs otherwise.
>
> For reference, this is the issue we are talking about:
> http://d.puremagic.com/issues/show_bug.cgi?id=3789
>
> The current situation is totally not acceptable. Even doing 
> what I think you are suggesting, that is generating a 
> compile-time error, is better than the current situation, that 
> leads to silent bugs.
>
> Bye,
> bearophile

Oh, thanks. Because we were talking about sorting, and you spoke 
of "comparison", I didn't get you meant comparison in the 
opEquals way (as opposed to equivalence).

Thanks for taking the time to look for the bug for me.
July 31, 2012
Re: sorting failed error
On Monday, 30 July 2012 at 22:23:10 UTC, Timon Gehr wrote:
> On 07/30/2012 03:52 PM, monarch_dodra wrote:
>> ...
>> NaN then compares false with everything, making it 
>> un-transitive, and
>> potentially breaking your cmp. COuld you try the algo with 
>> "return
>> 1/(1+travelled)".
>>
>> That, or because of the non-deterministic nature  of floating 
>> point
>> calculation,
>
> Floating-point computation is deterministic.

Is this specific to D? Because it isn't in C++:

http://www.parashift.com/c++-faq-lite/floating-point-arith2.html
July 31, 2012
Re: sorting failed error
I now tried to bring it to the next level, using concurrency to bread
a couple of populations and make the best solutions migrate between
them.
But, to use concurrency in D, one has to continually cast between
immutable, cast immutable away, ....
Not only that, casting away from immutable (or too, the error message
is not informative at all) you get error's along the line of "opEquals
doesn't work with immutable arguments"...

Why is it that cumbersome? It seems like D took a good idea and ruined
it with all that const-stuff.

I have the feeling that I've completely missed the whole idea on how
concurrency in D should work. Is it normal that you have to either
mess with immutable or with the broken shared?
Am I implementing everything wrong?
July 31, 2012
Re: sorting failed error
On Tuesday, July 31, 2012 11:41:19 maarten van damme wrote:
> I now tried to bring it to the next level, using concurrency to bread
> a couple of populations and make the best solutions migrate between
> them.
> But, to use concurrency in D, one has to continually cast between
> immutable, cast immutable away, ....
> Not only that, casting away from immutable (or too, the error message
> is not informative at all) you get error's along the line of "opEquals
> doesn't work with immutable arguments"...
> 
> Why is it that cumbersome? It seems like D took a good idea and ruined
> it with all that const-stuff.
> 
> I have the feeling that I've completely missed the whole idea on how
> concurrency in D should work. Is it normal that you have to either
> mess with immutable or with the broken shared?
> Am I implementing everything wrong?

To pass something across threads, it either needs to be a value type, or 
shared, or immutable. Unfortunately, shared doesn't work std.concurrency right 
now though, so you're stuck using immutable. What we really need is a way to 
indicate that ownership is being passed from one thread to another, but that 
can't really be done with the current type system. However, there are probably 
tweaks that can be made to make it more pleasant to deal with.

As for "messing it up with const", immutability (and therefore const) is a 
core part of D's threading model. By making everything thread-local by 
default, you then need a way to share instances across threads, and that 
requires shared (and immutable is implicitly shared). And with immutable, that 
can be done much more efficiently, because then the compiler doesn't have to 
worry about multiple threads screwing with the variable.

So, the situation isn't perfect and still needs some work, but the basic idea 
is solid and does work, even if it deosn't yet work quite as well as we'd 
like.

- Jonathan M Davis
July 31, 2012
Re: sorting failed error
On Tuesday, 31 July 2012 at 09:41:31 UTC, maarten van damme wrote:
> I now tried to bring it to the next level, using concurrency to 
> bread
> a couple of populations and make the best solutions migrate 
> between
> them.
> But, to use concurrency in D, one has to continually cast 
> between
> immutable, cast immutable away, ....
> Not only that, casting away from immutable (or too, the error 
> message
> is not informative at all) you get error's along the line of 
> "opEquals
> doesn't work with immutable arguments"...
>
> Why is it that cumbersome? It seems like D took a good idea and 
> ruined
> it with all that const-stuff.
>
> I have the feeling that I've completely missed the whole idea 
> on how
> concurrency in D should work. Is it normal that you have to 
> either
> mess with immutable or with the broken shared?
> Am I implementing everything wrong?

Use std.parallelism. It has everything that (I think) you need, 
e.g. support for parallel for, tasks and more.
Next ›   Last »
1 2 3
Top | Discussion index | About this forum | D home