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

simplify spread_spores and nested functions #37

Closed
PaulMelloy opened this issue Jan 15, 2021 · 10 comments
Closed

simplify spread_spores and nested functions #37

PaulMelloy opened this issue Jan 15, 2021 · 10 comments

Comments

@PaulMelloy
Copy link
Collaborator

I am wondering if we need to have all the following functions nested within the function spread_spores. It might be easier to code it all into fewer nestings and cut down on passing parameters from the top function to the bottom function.

Nesting as follows
trace_asco
-> one_day
->-> spread_spores
->->-> interception_probability
->->-> spores_each_wet_hour
->->->-> spores_from_1_element
->->->->-> potentially_effective_spores
->->->->-> wind_distance
->->->->-> splash_distance
->->->->-> splash_angle
->->->->-> address_from_center_distance
->->->->-> adjust_for_interception
->->->->->-> successful_infections
->->->->->->-> interception_probability
->->->->->->-> random_integer_from_real

each "->" indicates how many functions the named function is nested in
TBC....

@IhsanKhaliq
Copy link
Owner

Are you combining events in one_day function? If so, all these events can't possibly happen in a day. I checked the functions code, they seem logical in this order. Also, in the field spores are spread from infested stubble by rain splash and then they fall on host/get intercepted and finally infect. I am not if random integer from real is important. spores_each_wet_hour ->-> potentially_effective_spores ->-> spores_from_1_element ->-> splash_distance->-> splash_angle ->-> wind_distance ->-> wind_angle ->-> address_from_center_distance -> -> address_from_center_distance ->-> interception_probability ->-> adjust_for_interception ->-> successful_infections . I think it might be easier to divide functions based on

@IhsanKhaliq
Copy link
Owner

Or you meant one_season function?

@PaulMelloy
Copy link
Collaborator Author

I mean to keep most of the functions but organise them so they are not nested to the extent they currently are. So dropping down 3-4 layers of functions instead of the 7 as partially described above I think this will make it easier to maintain and make the code more readable.
Ultimately the model will be run with one function. I think it would be easier to run the season at the first function level, changing one_day to a loop function that apply a grow function, then disperse function, then interception/infection function in order. That would reduce nesting from ~ 7 to 4-5. Which, in my opinion, would be more ideal, easier to follow and debug.

ie something like this
Old
Season
-> day_in_season
->-> hour_in_day
->->-> grid_element()
->->->-> growing_point()
-> day_in_season
->-> hour_in_day
->->-> grid_element()
->->->-> disperse_spores()
-> day_in_season
->-> hour_in_day
->->-> grid_element()
->->->-> intercept_and_infect()

New
trace_asco (Season)
loop_day{
-> apply(grid_elements , grow_plants())
-> apply(grid_elements , disperse_infected()
-> apply(grid_elements , interception_infect()
}

@IhsanKhaliq
Copy link
Owner

Okay. I'd change infected to spores. That is, spread_spores and intercept_spores because spores are being spread and intercepted (disease can't possibly be intercepted, once diseased, the story is over). That said, spore interception function may not be very relevant in our case as we're studying in splash dispersal within paddock, so spores should always fall on chickpea/host

@adamhsparks
Copy link
Collaborator

lintr is a good package to use to help you sort out the complexity of the functions, the cyclomatic complexity can be calculated and you can then gauge from there what could improve the code: https://github.com/jimhester/lintr

@PaulMelloy
Copy link
Collaborator Author

Thanks Adam!

@PaulMelloy
Copy link
Collaborator Author

@IhsanKhaliq can you post a link to the graphic you have of the model please

@IhsanKhaliq
Copy link
Owner

02whole-pages-145-189.pdf

https://digital.library.adelaide.edu.au/dspace/bitstream/2440/75702/8/02whole.pdf Graphics are at the end of chapter 7 for of this thesis. I have already cut chapter 7 from the whole thesis for convenience

@IhsanKhaliq
Copy link
Owner

Screen Shot 2021-02-15 at 1 44 16 pm
Please see different color elipses representing different parameters

@IhsanKhaliq
Copy link
Owner

Done in trace_asco() & `one_day()" function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants