gama-platform / gama

Main repository for developing the 2024+ versions of GAMA
https://gama-platform.org
GNU General Public License v3.0
16 stars 5 forks source link

the `every` operator is highly inefficient #271

Open lesquoyb opened 1 month ago

lesquoyb commented 1 month ago

Describe the bug While trying to understand why a model was getting slower and slower over time I realized that after running it for one hour, most of the computing time was spent in the every operator. Looking at the code it makes sense as it seem to increment a duration in a loop until it reaches current date, so the farthest away the current date the longer it takes.

Additionally I didn't find any use for that kind of incremental code as the operator is not taking into account the particularities of incrementing one month (which could have a different number of days) as can be demonstrated by this model:

model every

global {

    float step <- 1#day;

    reflex t when:every(1#month){
        write current_date;
    }

    reflex y when:every(1#year){
        write "year " + current_date;
    }

    reflex end when:current_date = starting_date + 400#day{
        do die;
    }
}

experiment a;

which yields this result:

1970-01-01 07:00:00
year 1970-01-01 07:00:00
1970-01-30 07:00:00
1970-02-26 07:00:00
1970-03-04 07:00:00
1970-04-01 07:00:00
1970-05-05 07:00:00
1970-05-31 07:00:00
1970-06-30 07:00:00
1970-07-06 07:00:00
1970-08-06 07:00:00
1970-09-28 07:00:00
1970-10-07 07:00:00
1970-11-27 07:00:00
1970-12-08 07:00:00
year 1971-01-01 07:00:00
1971-01-08 07:00:00

Expected behavior every shouldn't slow down the model

AlexisDrogoul commented 1 month ago

So the bug is not in the slowdown but in the fact that every(1#month) does not work as expected, right ?

lesquoyb commented 1 month ago

no no it does slow down exponentially which is unexpected. The model was just to illustrate that whatever the code behind every is doing is not taking into account months and years properly anyway so we might as well change it to something more straightforward and equivalent to current_date - starting_date mod interval = 0

AlexisDrogoul commented 1 month ago

OK, but the issue here is not the slowing down. It's the fact that every does not work anymore with irregular intervals (it used to at some point in time). So replacing it with a version that is accordingly faster (i.e. because it does not care about the Nth months, as it considers that a month is 30 days and a year 12 months) but still buggy is a strange way to "solve" it.

ptaillandier commented 1 month ago

For information: I am currently working on a version that has exactly the same behavior as the current one (i.e. your version) but without the drawbacks. I hope to commit soon

Le mar. 30 juil. 2024 à 11:27, Alexis Drogoul @.***> a écrit :

OK, but the issue here is not the slowing down. It's the fact that every does not work anymore with irregular intervals (it used to at some point in time). So replacing it with a version that is accordingly faster (i.e. because it does not care about the Nth months, as it considers that a month is 30 days and a year 12 months) but still buggy is a strange way to "solve" it.

— Reply to this email directly, view it on GitHub https://github.com/gama-platform/gama/issues/271#issuecomment-2257436468, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALPWHLI7ZB67HCJ7GV2WATZO4I47AVCNFSM6AAAAABLVN4I36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJXGQZTMNBWHA . You are receiving this because you commented.Message ID: @.***>

AlexisDrogoul commented 1 month ago

So what needs to be addressed is the exponential slowing down + the correctness of every. I.e. find an algorithm that reinstalls the correct behavior (why has it disappeared in the first place ?) without the slowdown. It should be feasible !

lesquoyb commented 1 month ago

I think there's a problem of definition of the operator to begin with, so we should probably take some time to sit together and properly define what it's supposed to do in what case. Because there are multiple ways that could make sense depending on what we want this operator to be used for

AlexisDrogoul commented 1 month ago

The behavior should be simple : return true every time we reach the end of a period/interval (like a modulo); and the period/interval can be either regular (1, 1#s, etc.) or irregular (1#month), in which case it should depend on the current date (i.e. 2 months from the 2nd of February of 1969 will not result in the same interval as 2 months from the 2nd of August 1981).

ptaillandier commented 1 month ago

I commited a solution. I'll let you test.

Le mar. 30 juil. 2024 à 11:46, Baptiste Lesquoy @.***> a écrit :

I think there's a problem of definition of the operator to begin with, so we should probably take some time to sit together and properly define what it's supposed to do in what case. Because there are multiple ways that could make sense depending on what we want this operator to be used for

— Reply to this email directly, view it on GitHub https://github.com/gama-platform/gama/issues/271#issuecomment-2257452270, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALPWHKUD27SENEZPY6FL2LZO4LA7AVCNFSM6AAAAABLVN4I36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJXGQ2TEMRXGA . You are receiving this because you commented.Message ID: @.***>