Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow cover crops, multiple cropping and crop rotations #1500

Open
billsacks opened this issue Sep 24, 2021 · 6 comments
Open

Allow cover crops, multiple cropping and crop rotations #1500

billsacks opened this issue Sep 24, 2021 · 6 comments
Labels
enhancement new capability or improved behavior of existing capability science Enhancement to or bug impacting science

Comments

@billsacks
Copy link
Member

@danicalombardozzi @samsrabin and I talked briefly today about what might be needed to allow cover crops, multiple cropping and crop rotations. Although this isn't high priority, I thought it would be useful to have an issue to record any thoughts on the best way to implement these related enhancements.

@billsacks billsacks added enhancement new capability or improved behavior of existing capability tag: enh - new science labels Sep 24, 2021
@billsacks
Copy link
Member Author

Here are some notes from a meeting in 2015 about cover cropping:

Could you do this by switching the pft type on the column to generic crop part-way through the year?

Dave suggested having a secondary ivt that is applied in the off-season.

I also wondered if it's possible to have a second pft on the same soil column – and the weights of the two would trade off. i.e., this would be similar to what we do for natural veg PFTs. The general feeling was that that might make sense, but may be harder than Dave's idea of having a secondary ivt associated with the same p index.

One of the big considerations with all of these approaches is what happens with history variables. That was part of the motivation for Dave's suggestion: a given p would always dictate the same pft type to the history file, but it would have a secondary ivt for the sake of parameterizations.

@samsrabin
Copy link
Collaborator

Sam Levis left some notes in CropPhenology()—at the beginning of the "should we plant winter cereals today" if-block—that relate to what would need to change in the code in order to enable crop rotation.

! add check to only plant winter cereal after other crops (soybean, maize)
! have been harvested

! *** remember order of planting is crucial - in terms of which crops you want
! to be grown in what order ***

! in this case, corn or soybeans are assumed to be planted before
! cereal would be in any particular year that both patches are allowed
! to grow in the same grid cell (e.g., double-cropping)

! slevis: harvdate below needs cropplant(p) above to be cropplant(p,ivt(p))
!         where ivt(p) has rotated to winter cereal because
!         cropplant through the end of the year for a harvested crop.
!         Also harvdate(p) should be harvdate(p,ivt(p)) and should be
!         updated on Jan 1st instead of at harvest (slevis)

In the absence of a system for crop rotation, cropplant is only used to prevent a crop from being re-planted in a given year after it's already been harvested that year (with the beginning of the "year" differing between hemispheres).

In code I'm writing now (re: #519), this function of cropplant is redundant with a new counter variable crop_inst%growingseason_count that tracks the number of growing seasons that have begun in a given calendar year (i.e., since 1 Jan 00:00). I was thus planning to get rid of cropplant, but since there's an inkling that it might be useful for crop rotations, I'll leave it in.

To whomever ends up eventually implementing crop rotation: Presuming my code for #519 has been merged in, consider whether cropplant can be removed.

@billsacks
Copy link
Member Author

@samsrabin thanks for your careful look through the code in this respect as well as your detailed comment. Without having looked very closely, my feeling here is that the current code is probably so far from working in this respect that there isn't huge value in keeping cropplant around just for that inkling that it might be useful, and it seems confusing to have this extra, redundant logic. So I would recommend removing cropplant as you were originally intending. An ideal compromise would be to do that removal in a single commit (with nothing else in that commit). Then you could point to that commit hash in this issue as a guide in case anyone ever wanted to resurrect cropplant for this purpose.

@samsrabin
Copy link
Collaborator

I was kinda hoping you'd say that ;-). Done!

@billsacks
Copy link
Member Author

billsacks commented Oct 26, 2021

Thanks. If you ended up doing it in a self-contained commit, it could help to reference the commit hash here in case we want to look back at it. (If you haven't done this before, see https://docs.github.com/en/github/writing-on-github/working-with-advanced-formatting/autolinked-references-and-urls#commit-shas for the syntax for referencing a commit hash on your own fork, once you push it there.) (If not, no worries: I don't feel it's that important to have a reference to a self-contained commit for this removal.)

@danicalombardozzi
Copy link
Contributor

Thanks Sam for raising this issue and Bill for your recommendation!

samsrabin added a commit to samsrabin/CTSM that referenced this issue Oct 26, 2021
In the absence of a system for crop rotation, cropplant was only used to prevent a crop from being re-planted in a given year after it's already been harvested that year (with the beginning of the "year" differing between hemispheres). This is redundant with my new variable crop_inst%growingseason_count, which tracks the number of growing seasons that have begun in a given calendar year (i.e., since 1 Jan 00:00).

See discussion at ESCOMP#1500 (ESCOMP#1500).
samsrabin added a commit to samsrabin/CTSM that referenced this issue Jan 26, 2022
In the absence of a system for crop rotation, cropplant was only used to prevent a crop from being re-planted in a given year after it's already been harvested that year (with the beginning of the "year" differing between hemispheres). This is redundant with my new variable crop_inst%growingseason_count, which tracks the number of growing seasons that have begun in a given calendar year (i.e., since 1 Jan 00:00).

See discussion at ESCOMP#1500 (ESCOMP#1500).

(cherry picked from commit 036778a)
billsacks added a commit that referenced this issue Mar 28, 2022
Add outputs for annual crop sowing and harvest dates

Added annual outputs of sowing and harvest dates (`SDATES` and `HDATES`,
respectively). This should simplify the determination of sowing and
harvest date for postprocessing.
- Sowing dates are on new dimension `mxgrowseas` (maximum number of
  growing seasons allowed to begin in a year; currently hard-coded to 1).
- Harvest dates are on new dimension `mxharvests` (maximum number of
  harvests allowed in a year), which is `mxgrowseas`+1. This is needed
  because in year Y you might harvest a field that was planted in year
  Y-1, then plant and harvest again.
- The lengths of these dimensions are public constants of `clm_varpar`.

Additionally, removed `cropplant` as discussed here:
<#1500 (comment)>.

The `mxharvests` concept enables the addition of more such outputs to
further simplify crop postprocessing—for example, yield per growing
season as a direct output rather than needing to cross-reference daily
grain mass against the day of harvest.

These changes involved some rework to the crop phenology code that
changes answers for crop phenology in some circumstances. All answer
changes were determined to be due to issues / oddities in the old code.
See discussion in #1616 for details
(especially see
#1616 (comment)). To
summarize briefly, some issues with the old code were:
- Non-winter-cereal patches that had live crops at the beginning of the
  year did not get planted later that year.
- There was some odd behavior for rice patches at exactly 0 deg latitude
- Crop root depth had unexpected values outside the growing season; now
  root depth is set to 0 outside the growing season

Resolves #1537 (Output of sowing and harvest dates)
samsrabin added a commit to samsrabin/CTSM that referenced this issue Mar 28, 2022
Add outputs for annual crop sowing and harvest dates

Added annual outputs of sowing and harvest dates (`SDATES` and `HDATES`,
respectively). This should simplify the determination of sowing and
harvest date for postprocessing.
- Sowing dates are on new dimension `mxgrowseas` (maximum number of
  growing seasons allowed to begin in a year; currently hard-coded to 1).
- Harvest dates are on new dimension `mxharvests` (maximum number of
  harvests allowed in a year), which is `mxgrowseas`+1. This is needed
  because in year Y you might harvest a field that was planted in year
  Y-1, then plant and harvest again.
- The lengths of these dimensions are public constants of `clm_varpar`.

Additionally, removed `cropplant` as discussed here:
<ESCOMP#1500 (comment)>.

The `mxharvests` concept enables the addition of more such outputs to
further simplify crop postprocessing—for example, yield per growing
season as a direct output rather than needing to cross-reference daily
grain mass against the day of harvest.

These changes involved some rework to the crop phenology code that
changes answers for crop phenology in some circumstances. All answer
changes were determined to be due to issues / oddities in the old code.
See discussion in ESCOMP#1616 for details
(especially see
ESCOMP#1616 (comment)). To
summarize briefly, some issues with the old code were:
- Non-winter-cereal patches that had live crops at the beginning of the
  year did not get planted later that year.
- There was some odd behavior for rice patches at exactly 0 deg latitude
- Crop root depth had unexpected values outside the growing season; now
  root depth is set to 0 outside the growing season
slevis-lmwg added a commit to slevis-lmwg/ctsm that referenced this issue Mar 29, 2022
Add outputs for annual crop sowing and harvest dates

Added annual outputs of sowing and harvest dates (`SDATES` and `HDATES`,
respectively). This should simplify the determination of sowing and
harvest date for postprocessing.
- Sowing dates are on new dimension `mxgrowseas` (maximum number of
  growing seasons allowed to begin in a year; currently hard-coded to 1).
- Harvest dates are on new dimension `mxharvests` (maximum number of
  harvests allowed in a year), which is `mxgrowseas`+1. This is needed
  because in year Y you might harvest a field that was planted in year
  Y-1, then plant and harvest again.
- The lengths of these dimensions are public constants of `clm_varpar`.

Additionally, removed `cropplant` as discussed here:
<ESCOMP#1500 (comment)>.

The `mxharvests` concept enables the addition of more such outputs to
further simplify crop postprocessing—for example, yield per growing
season as a direct output rather than needing to cross-reference daily
grain mass against the day of harvest.

These changes involved some rework to the crop phenology code that
changes answers for crop phenology in some circumstances. All answer
changes were determined to be due to issues / oddities in the old code.
See discussion in ESCOMP#1616 for details
(especially see
ESCOMP#1616 (comment)). To
summarize briefly, some issues with the old code were:
- Non-winter-cereal patches that had live crops at the beginning of the
  year did not get planted later that year.
- There was some odd behavior for rice patches at exactly 0 deg latitude
- Crop root depth had unexpected values outside the growing season; now
  root depth is set to 0 outside the growing season
slevis-lmwg added a commit that referenced this issue Mar 29, 2022
Add outputs for annual crop sowing and harvest dates

Added annual outputs of sowing and harvest dates (`SDATES` and `HDATES`,
respectively). This should simplify the determination of sowing and
harvest date for postprocessing.
- Sowing dates are on new dimension `mxgrowseas` (maximum number of
  growing seasons allowed to begin in a year; currently hard-coded to 1).
- Harvest dates are on new dimension `mxharvests` (maximum number of
  harvests allowed in a year), which is `mxgrowseas`+1. This is needed
  because in year Y you might harvest a field that was planted in year
  Y-1, then plant and harvest again.
- The lengths of these dimensions are public constants of `clm_varpar`.

Additionally, removed `cropplant` as discussed here:
<#1500 (comment)>.

The `mxharvests` concept enables the addition of more such outputs to
further simplify crop postprocessing—for example, yield per growing
season as a direct output rather than needing to cross-reference daily
grain mass against the day of harvest.

These changes involved some rework to the crop phenology code that
changes answers for crop phenology in some circumstances. All answer
changes were determined to be due to issues / oddities in the old code.
See discussion in #1616 for details
(especially see
#1616 (comment)). To
summarize briefly, some issues with the old code were:
- Non-winter-cereal patches that had live crops at the beginning of the
  year did not get planted later that year.
- There was some odd behavior for rice patches at exactly 0 deg latitude
- Crop root depth had unexpected values outside the growing season; now
  root depth is set to 0 outside the growing season
ekluzek added a commit to ekluzek/CTSM that referenced this issue Jul 14, 2022
Add outputs for annual crop sowing and harvest dates

Added annual outputs of sowing and harvest dates (`SDATES` and `HDATES`,
respectively). This should simplify the determination of sowing and
harvest date for postprocessing.
- Sowing dates are on new dimension `mxgrowseas` (maximum number of
  growing seasons allowed to begin in a year; currently hard-coded to 1).
- Harvest dates are on new dimension `mxharvests` (maximum number of
  harvests allowed in a year), which is `mxgrowseas`+1. This is needed
  because in year Y you might harvest a field that was planted in year
  Y-1, then plant and harvest again.
- The lengths of these dimensions are public constants of `clm_varpar`.

Additionally, removed `cropplant` as discussed here:
<ESCOMP#1500 (comment)>.

The `mxharvests` concept enables the addition of more such outputs to
further simplify crop postprocessing—for example, yield per growing
season as a direct output rather than needing to cross-reference daily
grain mass against the day of harvest.

These changes involved some rework to the crop phenology code that
changes answers for crop phenology in some circumstances. All answer
changes were determined to be due to issues / oddities in the old code.
See discussion in ESCOMP#1616 for details
(especially see
ESCOMP#1616 (comment)). To
summarize briefly, some issues with the old code were:
- Non-winter-cereal patches that had live crops at the beginning of the
  year did not get planted later that year.
- There was some odd behavior for rice patches at exactly 0 deg latitude
- Crop root depth had unexpected values outside the growing season; now
  root depth is set to 0 outside the growing season
@samsrabin samsrabin added science Enhancement to or bug impacting science and removed enh - new science labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new capability or improved behavior of existing capability science Enhancement to or bug impacting science
Projects
None yet
Development

No branches or pull requests

3 participants