Skip to content

Move segmented restart functions to PEcAn.SIPNET#4008

Open
infotroph wants to merge 7 commits into
PecanProject:developfrom
infotroph:move-sipnet-restart
Open

Move segmented restart functions to PEcAn.SIPNET#4008
infotroph wants to merge 7 commits into
PecanProject:developfrom
infotroph:move-sipnet-restart

Conversation

@infotroph
Copy link
Copy Markdown
Member

  • Relocates write_segmented_configs.SIPNET for use without needing to source it in separately
  • Adds tests
  • Fixes a buglet from dropping dimension when ensemble.samples contains only one trait

Copy link
Copy Markdown
Member

@divine7022 divine7022 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!, couple of nice to have test
but none are blockers

@@ -0,0 +1,100 @@
test_that("write_segmented_configs", {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few this here
1 ) ryt now test only hits CSV path in segment_dataframe, but event_json fallback case is still not covered
so could be a second test_that block that omits crop_changes and provides an event_json json ?
2 ) zit worth having a test to varify RESTART_IN / RESTART_OUT chain inside the per segment sipnet.in files ? since the chain is whole point of segmented restarts, tiny check that segment 2's sipnet.in references segment 1's restart.out (and so on) could help lock in the core behaviour

up to you whether these go in this PR or followup. either is fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants