traitecoevo / plant

Trait-Driven Models of Ecology and Evolution :evergreen_tree:

https://traitecoevo.github.io/plant

53 stars 20 forks source link

Refactor control #314

Closed aornugent closed 2 years ago

aornugent commented 2 years ago

One step closer to #306.

This removes the Control object from Parameters and passes it directly to SCM or Patch APIs as required. This is handled on the R side with what I hope are sensible defaults, so most often users won't notice it's even there.

result <- run_scm_collect(p = expand_parameters(p0, trait_matrix), 
                          control = scm_base_control())

All tests pass, but I didn't test the carrying capacity or equilibrium scripts as I'm not totally sure what the correct output should be. Please let me know if there's anything that looks like it needs further investigation.

Next step will be repeating this exercise with Environment.

codecov-commenter commented 2 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@4c68d6f). Click here to learn what that means. The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop     #314   +/-   ##
==========================================
  Coverage           ?   79.84%           
==========================================
  Files              ?       94           
  Lines              ?     8400           
  Branches           ?        0           
==========================================
  Hits               ?     6707           
  Misses             ?     1693           
  Partials           ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4c68d6f...506586b. Read the comment docs.

dfalster commented 2 years ago

This looks great, well done @aornugent

I didn't test the carrying capacity or equilibrium scripts as I'm not totally sure what the correct output should be. Please let me know if there's anything that looks like it needs further investigation.

That's OK, I can review those. But that won't happen today. So feel free to merge, move on with environment, I can get back to you about that soon. Please assign me a GH issue