Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the every operator is highly inefficient #271

Closed
lesquoyb opened this issue Jul 30, 2024 · 8 comments
Closed

the every operator is highly inefficient #271

lesquoyb opened this issue Jul 30, 2024 · 8 comments
Labels
About GAML This issue concerns the GAML language 🤗 Enhancement This is a request for enhancement 👍 Fix to be tested

Comments

@lesquoyb
Copy link
Contributor

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

@lesquoyb lesquoyb added the 🤗 Enhancement This is a request for enhancement label Jul 30, 2024
ptaillandier added a commit that referenced this issue Jul 30, 2024
@AlexisDrogoul
Copy link
Member

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

@lesquoyb
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor

ptaillandier commented Jul 30, 2024 via email

@AlexisDrogoul
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor

ptaillandier commented Jul 30, 2024 via email

@lesquoyb lesquoyb changed the title the every operator is highly ineffective the every operator is highly inefficient Jul 31, 2024
@AlexisDrogoul AlexisDrogoul added About GAML This issue concerns the GAML language 👍 Fix to be tested labels Aug 31, 2024
@lesquoyb lesquoyb moved this from Todo to To Test in GAMA 2024-11 Sep 16, 2024
@github-project-automation github-project-automation bot moved this from To Test to Done in GAMA 2024-11 Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
About GAML This issue concerns the GAML language 🤗 Enhancement This is a request for enhancement 👍 Fix to be tested
Projects
Status: Done
Development

No branches or pull requests

3 participants