fix feasibilty and slow convergences issues related bii_target_apr24

[flohump]

🐦 Description of this PR 🐦

This PR aims to fix feasibilty and slow convergences issues related to modules/44_biodiversity/bii_target_apr24

• The calculation of i44_biome_share(j,biome44) has been revised to avoid very small numbers.
• The linear time interpolation for p44_bii_lower_bound(t2,i,biome44) was executed even if s44_bii_lower_bound = 0, which is not meaningful and rather confusing. With this PR the calculation is only excuted if s44_bii_lower_bound > 0.
• Based on recommendations from the CONOPT support the equation q44_bii is scaled to avoid very small numbers.
• The FSEC start scripts needs more than 1 h for starting all runs, especially if HR runs are started in-between. Therefore, the option lock_timeout = 6 is used.

This PR has been sucessfully tested with with default test runs (test_runs.R) and all FSEC (c200 and c1000) test runs.

Changelog

• 44_biodiversity bugfix i44_biome_share, code cleanup, added scaling of q44_bii
• start_scripts added lock_timeout as option to start_run function

🔧 Checklist for PR creator 🔧

• Label pull request from the label list.

  • Low risk: Simple bugfixes (missing files, updated documentation, typos) or changes in start or output scripts
  • Medium risk: Uncritical changes in the model core (e.g. moderate modifications in non-default realizations)
  • High risk: Critical changes in model core or default settings (e.g. changing a model default or adjusting a core mechanic in the model)

• Self-review own code

  • No hard coded numbers and cluster/country/region names.
  • The new code doesn't contain declared but unused parameters or variables.
  • magpie4 R library has been updated accordingly and backwards compatible where necessary.
  • scenario_config.csv has been updated accordingly (important if default.cfg has been updated)

• Document changes

  • Add changes to CHANGELOG.md
  • Where relevant, put In-code documentation comments
  • Properly address updates in interfaces in the module documentations
  • run goxygen::goxygen() and verify the modified code is properly documented

• Perform test runs

  • Low risk:
    • Run a compilation check via Rscript start.R --> "compilation check"
  • Medium risk:
    • Run test runs via Rscript start.R --> "test runs"
    • Check logs for errors/warnings
  • High risk:
    • Run test runs via Rscript start.R --> "test runs"
    • Check logs for errors/warnings
    • Default run from the PR target branch for comparison
    • Provide relevant comparison plots (land-use, emissions, food prices, land-use intensity,...)

📉 Performance changes 📈

• Current develop branch default : 25 mins
• This PR's default : 30 mins

🚨 Checklist for reviewer 🚨

• PR is labeled correctly
• Code changes look reasonable
  • No hard coded numbers and cluster/country/region names.
  • No unnecessary increase in module interfaces
  • model behavior/performance is satisfactory.
• Changes are properly documented
  • CHANGELOG is updated correctly
  • Updates in interfaces have been properly addressed in the module documentations
  • In-code documentation looks appropriate
• content review done (at least 1)
• RSE review done (at least 1)

[emolinab]

Why is a lock_timeout of six hours needed?

[flohump]

Because the FSEC start script starts many runs with c200 and in addition for all runs a c1000 high resolution run is started. If some c200 runs finish before all runs are submitted (what happend in my tests), the starting of c1000 runs locks the model folder and delays the start of the remaining c200 runs. Therefore, a longer lock_timeout is needed.

[emolinab]

Pat will probably have the key input for the biodiversity changes. Since I only partially understand what you did besides the scaling and clean-up, I have a few understanding questions. Are the changes made in the biodiversity module related to what you mentioned about having a too low boundary for vm_bv? If so, How is this associated with removing p44_target_value(i,biome44)?

[flohump]

I cleaned-up the code for the linear interpolation because it was confusing. p44_target_value(i,biome44) is just no longer needed.

[pvjeetze]

Thanks a lot for these fixes! I only have a brief clarification question.

[pvjeetze]

Why is the scaling needed here?

[flohump]

This is based on recommendations from the CONOPT support. Without the scaling, some test runs are infeasible.