search

forsys-sp / forsysr

An R implementation of the ForSys program
GNU General Public License v3.0
8 stars 3 forks source link

Move annual limits into selection functions #91

Closed codyevers closed 1 year ago

codyevers commented 1 year ago

In order to improve organization of the code and help improve it's generalizabilty, I'm moving the annual target functionality inside of the build_static_projects and build_dynamic_projects and renaming these to a more general parameter name project_ceiling. In part this is because the two selection functions work differently... the former largely by grouping and summing, the latter by looping.

This will meaning changing the patchmax project building to while (e.g., projects < count & ceiling < limit), which will allow us to either build a certain number of patches or patches up to some limit (e.g., area, budget, etc.).

These changes prevents the celing from only related to some annual sequence, and therefore avoids confusion with ETrt_YR. That is, ETrt_YR is an specific example of implementing an overall project ceiling, but not the only one.

codyevers commented 1 year ago

@michelledayusfs, can you do a quick double-check to make sure the annual target makes sense as I have set up and seems to work correctly. Thanks!

michelledayusfs commented 1 year ago

I think this might be what is erroring out in patchmax. I'll test it with predefined planning areas.

michelledayusfs commented 1 year ago

To be consistent we should use 'annual_target_field' AND 'annual_target_value' rather than 'anual_target'. Note our syntax with 'project_target_field' and 'project_target_value', but also let's discuss terminology here. I'm not sure annual is intuitive.

michelledayusfs commented 1 year ago

Oh- it requires you to fill out this parameter? planning_years or defaults to 1.

(That's right-- she looking at the code.)

michelledayusfs commented 1 year ago

Seems to work as intended with predefined planning areas.

Testing_ceiling_predfined.txt

codyevers commented 1 year ago

@michelledayusfs I changed annual_target to annual_target_value per your suggestions.

Inside of forsys, I've changed these fields to global_target_field and global_target_value.

Should I change these names for forsys::run as well?

One idea would be to call these:

Is this more clear? Let's discuss!

michelledayusfs commented 1 year ago

I think as we discussed we will leave it as is for now. The planning_years parameters clarifies things.