Skip to content

sipnet: drop segments with unchanged PFT - #4129

Merged
infotroph merged 9 commits into
PecanProject:developfrom
tyagivibhor111-hub:fix/sipnet-drop-same-pft-segments
Oct 23, 2026
Merged

infotroph merged 9 commits into
PecanProject:developfrom
tyagivibhor111-hub:fix/sipnet-drop-same-pft-segments

Conversation

@tyagivibhor111-hub

Copy link
Copy Markdown
Contributor

Description

Updated write_segmented_configs() so that consecutive crop segments
that map to the same PFT are merged instead of creating separate SIPNET
segments.

The segment boundaries are checked after the crop-code-to-PFT mapping,
and unnecessary transitions are removed. The retained segments continue
from the correct start time.

Motivation and Context

When different crop codes map to the same PFT, write_segmented_configs()
previously created separate segments and unnecessary SIPNET restarts.

This change avoids those unnecessary restarts when the PFT does not
actually change.

Fixes #4122

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes issue sipnet: Drop same-pft segments #4122)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tyagivibhor111-hub

Copy link
Copy Markdown
Contributor Author

Hi @infotroph, could you please check my PR #4129 matches the intended solution and let me know if any changes are needed?

@tyagivibhor111-hub

Copy link
Copy Markdown
Contributor Author

Hey @infotroph ,I’ve made the requested changes: used dplyr::consecutive_id() to merge consecutive segments with the same PFT, updated the pipeline style, formatted the fixture CSV values, and added a unit test for drop_same_pft_segments(). I also added a Windows fallback to copy job_segmented.sh to job.sh when symlink creation fails. Both the focused test and full SIPNET test suite pass locally. Thanks for the suggestions and If there any changes required than you can tell!

Co-authored-by: Chris Black <chris@ckblack.org>
Co-authored-by: Chris Black <chris@ckblack.org>

@infotroph infotroph left a comment

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.

The code looks good now, thanks. One more request I neglected earlier: Can you add a line to NEWS.md?

@tyagivibhor111-hub

tyagivibhor111-hub commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

The code looks good now, thanks. One more request I neglected earlier: Can you add a line to NEWS.md?

Yaa Sure @infotroph, I add a Line in NEWS.md.
If you were Free, Could you please review my PR when you get a chance? #4163

@infotroph
infotroph added this pull request to the merge queue Oct 23, 2026
Merged via the queue into PecanProject:develop with commit 0cad991 Oct 23, 2026
22 checks passed
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.

sipnet: Drop same-pft segments

2 participants