search

DIFM-Brain / ofpetrial

GNU General Public License v3.0
0 stars 1 forks source link

Respecting min and max plot arguments #48

Open brittanikedge opened 3 months ago

brittanikedge commented 3 months ago

get_trial_plot_data() does not always strictly follow the min and max plot length arguments

brittanikedge commented 3 months ago

@tmieno2 We noticed that the code does not strictly respect the min and max plot length arguments.

I did some rewriting of the current version of the function, but it's still not quite there. For example, when I have the min at 250 in my example, I still end up with a plot of 248. So I've been thinking about rewriting the whole thing. I've been looking at the function and noticed that it looks like there was an old version that used the remainder calculated.

I've been wondering why we don't we use the min to calculate the remainder and then divide up the remainder as much as we can while respecting the max length. I have a feeling this could also be done in fewer lines of code.

I wanted to ask you because it looks like you tried using the remainder previously based on the old function I see in the R script. Was there a particular problem that you ran into? I'm going to write up my attempt and see how it goes.

brittanikedge commented 3 months ago

@tmieno2 I just loaded my new version of the function to the dev folder. Let me know what you think!

tmieno2 commented 3 months ago

Okay, we should definitely get rid of the current one and use the new one for sure.

But, now I remember why I felt slightly uncomfortable with both the current and new ones. The old code we used to use before the package development, we tried to make the plot length the same as much as possible across the strips even though strip lengths vary so that plots lines up nicely (as long as the starting end of the field is straight and flat) while filling the entire strip as much as possible by changing the plot length of only the last few plots. But, it is not like I had any statistical reasons why we should do so. I though it would just look better (it is indeed help define block better for block designs, but we were not doing that before). Do you think some farmers, consultants, and researchers care about this?

brittanikedge commented 3 months ago

I think that was an issue when we weren't checking across the strips to see if there were repeated treatments side-by-side. Even with the attempt to make similar sizes, the shape of the field still caused some parts of the fields to have the same rates in adjacent plots. I haven't heard anyone complain about this in the past, even when there were irregular edges of the field that resulted in plots that didn't line up. I think it should be fine.

For those that want a regular grid, perhaps we should provide that option as well? As you said, I can see that option being for those who use a block design.

tmieno2 commented 3 months ago

You know what. Let's just wait until the complain comes up. Haha. For now, I replace the old code with yours and pushed them along with other fixes.

brittanikedge commented 3 months ago

I like it! We'll see what happens.