Add ammonia emissions#176
Conversation
🦋 Changeset detectedLatest commit: e6a4568 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded@SvenVw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis update introduces comprehensive support for calculating, displaying, and documenting ammonia (NH₃) emissions from fertilizer applications and crop residues. It expands the data model, database schema, and form validation to include new fertilizer parameters ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as fdm-app (UI)
participant API as fdm-core/fdm-calculator
participant DB as Database
User->>UI: Add/update fertilizer with p_no3_rt, p_nh4_rt, p_ef_nh3
UI->>API: Validate and submit fertilizer data
API->>DB: Store new fertilizer parameters
User->>UI: View nitrogen balance details
UI->>API: Request nitrogen balance calculation
API->>DB: Fetch cultivations, fertilizer applications, details
API->>API: Calculate ammonia volatilization (by fertilizer type)
API->>UI: Return nitrogen balance with ammonia emission breakdown
UI->>User: Display nested ammonia emission accordions and values
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #176 +/- ##
===============================================
+ Coverage 93.29% 93.45% +0.15%
===============================================
Files 68 73 +5
Lines 8384 8784 +400
Branches 999 1087 +88
===============================================
+ Hits 7822 8209 +387
- Misses 560 573 +13
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…he ammonia emission factor of a fertilizer
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/mineral.ts (1)
13-13: Fix remaining typo in documentation.The documentation still contains the typo "Emmission" that was flagged in previous reviews.
- * `determineMineralAmmoniaEmmissionFactor`), and calculates the ammonia emissions + * `determineMineralAmmoniaEmissionFactor`), and calculates the ammonia emissions
🧹 Nitpick comments (6)
.changeset/tasty-grapes-help.md (1)
5-5: Improve grammar for better readability.Consider adding the article "the" for improved readability.
-Add calculation of ammonia emissions to nitrogen balance calculation. +Add calculation of ammonia emissions to the nitrogen balance calculation..changeset/green-symbols-arrive.md (1)
5-5: Improve grammar for better readability.Consider adding the article "the" for improved readability.
-Add `p_no3_rt` and `p_nh4_rt` to SRM catalogue. +Add `p_no3_rt` and `p_nh4_rt` to the SRM catalogue..changeset/clean-rooms-post.md (1)
5-5: Improve grammar for better readability.Consider adding the article "the" for improved readability.
-Add parameter `p_ef_nh3` to the fertilizersCatalogue to represent the ammonia emission factor of a fertilizer. +Add parameter `p_ef_nh3` to the fertilizersCatalogue to represent the ammonia emission factor of a fertilizer.Wait, this line already has "the" before "fertilizersCatalogue" but is missing "the" before the second part. Let me correct:
-Add parameter `p_ef_nh3` to the fertilizersCatalogue to represent the ammonia emission factor of a fertilizer. +Add parameter `p_ef_nh3` to the fertilizersCatalogue to represent the ammonia emission factor of a fertilizer.Actually, looking more carefully, the issue is with the first "the":
-Add parameter `p_ef_nh3` to the fertilizersCatalogue to represent the ammonia emission factor of a fertilizer. +Add parameter `p_ef_nh3` to the fertilizersCatalogue to represent the ammonia emission factor of a fertilizer.fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.ts (1)
69-69: Fix misleading comment.The comment refers to "manure" but this function handles "other" fertilizer types.
- // Calculate for this application the amount of Nitrogen supplied by manure + // Calculate for this application the amount of Nitrogen supplied by other fertilizersfdm-docs/docs/balance/01-nitrogen.md (2)
207-265: Fix markdown formatting issues for consistency.The nested list indentation should follow a consistent 2-space pattern, and tables should be surrounded by blank lines.
Apply this diff to fix the formatting:
-* **Manure, Compost, and Other Organic Fertilizers:** - For these organic fertilizers, the emission is calculated based on the Total Ammoniacal Nitrogen (TAN) content, as this is the fraction of nitrogen that is readily available for volatilization. - - * **Formula:** - `NH3 Emission (kg N / ha) = Application Amount (kg / ha) * TAN Content (fraction) * Emission Factor (fraction)` - Where: - * `Application Amount`: `p_app_amount` (kg / ha) - The total amount of fertilizer applied. - * `TAN Content`: `p_nh4_rt` (fraction) - The fraction of total nitrogen that is in ammoniacal form. - * `Emission Factor`: A dimensionless factor representing the proportion of TAN that is volatilized as ammonia. This factor is determined by the application method and the type of land (grassland, cropland, or bare soil) at the time of application. - - * **Emission Factors for Manure and Compost:** +* **Manure, Compost, and Other Organic Fertilizers:** + For these organic fertilizers, the emission is calculated based on the Total Ammoniacal Nitrogen (TAN) content, as this is the fraction of nitrogen that is readily available for volatilization. + + * **Formula:** + `NH3 Emission (kg N / ha) = Application Amount (kg / ha) * TAN Content (fraction) * Emission Factor (fraction)` + Where: + * `Application Amount`: `p_app_amount` (kg / ha) - The total amount of fertilizer applied. + * `TAN Content`: `p_nh4_rt` (fraction) - The fraction of total nitrogen that is in ammoniacal form. + * `Emission Factor`: A dimensionless factor representing the proportion of TAN that is volatilized as ammonia. This factor is determined by the application method and the type of land (grassland, cropland, or bare soil) at the time of application. + + * **Emission Factors for Manure and Compost:** + | Application Method | Grassland | Cropland | Bare Soil | | :-------------------- | :-------- | :------- | :-------- |Similar indentation fixes should be applied to the mineral fertilizers section (lines 226-249) and crop residues section (lines 252-268).
257-257: Fix typo in emission factor formula.There's an extra backslash in the formula.
- `NH3 Emission (kg N / ha) = Residue N Content (kg N / ha) * \Emission Factor (fraction)` + `NH3 Emission (kg N / ha) = Residue N Content (kg N / ha) * Emission Factor (fraction)`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.changeset/clean-rooms-post.md(1 hunks).changeset/green-symbols-arrive.md(1 hunks).changeset/slow-parks-tickle.md(1 hunks).changeset/strong-bobcats-push.md(1 hunks).changeset/tasty-grapes-help.md(1 hunks).changeset/tiny-tires-tickle.md(1 hunks)fdm-app/app/components/blocks/fertilizer/formschema.tsx(2 hunks)fdm-calculator/src/balance/nitrogen/volatization/fertilizers/compost.ts(1 hunks)fdm-calculator/src/balance/nitrogen/volatization/fertilizers/index.ts(1 hunks)fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.test.ts(1 hunks)fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts(1 hunks)fdm-calculator/src/balance/nitrogen/volatization/fertilizers/mineral.ts(1 hunks)fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.test.ts(1 hunks)fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.ts(1 hunks)fdm-calculator/src/balance/nitrogen/volatization/index.ts(2 hunks)fdm-docs/docs/balance/01-nitrogen.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- .changeset/tiny-tires-tickle.md
- .changeset/strong-bobcats-push.md
- fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.test.ts
- fdm-calculator/src/balance/nitrogen/volatization/fertilizers/compost.ts
- fdm-calculator/src/balance/nitrogen/volatization/index.ts
- .changeset/slow-parks-tickle.md
- fdm-calculator/src/balance/nitrogen/volatization/fertilizers/index.ts
- fdm-app/app/components/blocks/fertilizer/formschema.tsx
- fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.test.ts
🧰 Additional context used
🧠 Learnings (1)
fdm-docs/docs/balance/01-nitrogen.md (1)
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
🪛 LanguageTool
fdm-docs/docs/balance/01-nitrogen.md
[uncategorized] ~211-~211: Loose punctuation mark.
Context: ...lizer applied. * TAN Content: p_nh4_rt (fraction) - The fraction of...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~212-~212: Loose punctuation mark.
Context: ...acal form. * Emission Factor: A dimensionless factor representing the...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~233-~233: Loose punctuation mark.
Context: ...(kg / ha). * Total N Content: p_n_rt (fraction). * `Emiss...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~234-~234: Loose punctuation mark.
Context: ...fraction). * Emission Factor: p_ef_nh3 (fraction). This factor can ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~240-~240: Loose punctuation mark.
Context: ..._rt - p_nh4_rt). * p_no3_rt: Nitrate content. * p_nh4_rt`...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~241-~241: Loose punctuation mark.
Context: ... Nitrate content. * p_nh4_rt: Ammonium content (TAN). * `p_...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~242-~242: Loose punctuation mark.
Context: ...nium content (TAN). * p_s_rt: Sulfur content. * K_1, `K_2...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~243-~243: Loose punctuation mark.
Context: ...s_rt: Sulfur content. * K_1, K_2, K_3`: Empirical constants. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~260-~260: Loose punctuation mark.
Context: ...onCatalogue). * Emission Factor`: This factor is calculated based on the ...
(UNLIKELY_OPENING_PUNCTUATION)
.changeset/clean-rooms-post.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: --- "@svenvw/fdm-core": minor --- Add parameter p_ef_nh3 to the fertilizersCatalogue ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
.changeset/green-symbols-arrive.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...r --- Add p_no3_rt and p_nh4_rt to SRM catalogue.
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
.changeset/tasty-grapes-help.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...@svenvw/fdm-calculator": minor --- Add calculation of ammonia emissions to nitrogen balanc...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...Add calculation of ammonia emissions to nitrogen balance calculation.
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
fdm-docs/docs/balance/01-nitrogen.md
207-207: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
210-210: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
211-211: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
212-212: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
215-215: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
229-229: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
232-232: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
233-233: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
234-234: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
239-239: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
240-240: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
241-241: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
242-242: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
243-243: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
244-244: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
245-245: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
246-246: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
247-247: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
259-259: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
260-260: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
262-262: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
265-265: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: core (22)
- GitHub Check: calculator (22)
🔇 Additional comments (2)
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/other.ts (1)
63-67: Clarify why manure emission factor is used for "other" fertilizers.The function uses
determineManureAmmoniaEmissionFactorfor "other" fertilizer types. This seems inconsistent unless "other" fertilizers are intended to use the same calculation method as manure.Please clarify if "other" fertilizer types should use the manure emission factor calculation or if they need their own specific calculation method.
fdm-calculator/src/balance/nitrogen/volatization/fertilizers/manure.ts (1)
1-193: Well-implemented ammonia emissions calculations for manure!The implementation correctly:
- Uses Decimal.js for precise calculations
- Properly determines land type (grassland/cropland/bare soil) at application time
- Applies the correct emission factors as documented
- Includes comprehensive error handling for unsupported application methods
- Has clear JSDoc comments explaining the functionality
|
|
||
| /** | ||
| * Determines the ammonia emission factor for mineral fertilizers based on their | ||
| * nitrogen, nitrate, ammonium, and sulfur content, and the presence of an inhibitor. |
There was a problem hiding this comment.
you only apply this function when p_ef_nh3 is null?
| Where: | ||
| * `Application Amount`: `p_app_amount` (kg / ha) - The total amount of fertilizer applied. | ||
| * `TAN Content`: `p_nh4_rt` (fraction) - The fraction of total nitrogen that is in ammoniacal form. | ||
| * `Emission Factor`: A dimensionless factor representing the proportion of TAN that is volatilized as ammonia. This factor is determined by the application method and the type of land (grassland, cropland, or bare soil) at the time of application. |
There was a problem hiding this comment.
TAN content is g/kg and not a fraction isn't?
|
|
||
| * **Emission Factors for Manure and Compost:** | ||
| | Application Method | Grassland | Cropland | Bare Soil | | ||
| | :-------------------- | :-------- | :------- | :-------- | |
There was a problem hiding this comment.
how is cropland versus bare soil defined? as manure data before sowing date or after harvest date then its a bare soil?
There was a problem hiding this comment.
Yes, when no cultivation is present, it is bare soil
| * `K_2 = -4.308 * 10^-5` | ||
| * `K_3 = 2.498 * 10^-4` | ||
| *Note: Currently, the presence of an inhibitor (`p_inhibitor`) is hardcoded to `false` in the calculation.* | ||
|
|
There was a problem hiding this comment.
interface does not allow user to select with/without inhibitor? thats fine, but good to add to action list
There was a problem hiding this comment.
Yes, inhibitor is not implement as parameter of fertilizer yet
gerardhros
left a comment
gerardhros
left a comment
There was a problem hiding this comment.
calculations fine. well done.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
fdm-docs/docs/balance/01-nitrogen.md (2)
257-258: Remove stray back-slash before “Emission Factor”The back-slash renders literally and clutters the formula.
-`NH3 Emission (kg N / ha) = Residue N Content (kg N / ha) * \Emission Factor (fraction)` +`NH3 Emission (kg N / ha) = Residue N Content (kg N / ha) * Emission Factor (fraction)`
214-223: Markdown lint: add blank lines around the emission-factor table
mdlintflags MD058; inserting an empty line before and after the table silences the warning and improves readability.- * **Emission Factors for Manure and Compost:** + * **Emission Factors for Manure and Compost:** + | Application Method | Grassland | Cropland | Bare Soil | | :-------------------- | :-------- | :------- | :-------- | @@ | Incorporation 2 Tracks| N/A | N/A | 0.46 | +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-docs/docs/balance/01-nitrogen.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
fdm-docs/docs/balance/01-nitrogen.md (1)
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
🪛 LanguageTool
fdm-docs/docs/balance/01-nitrogen.md
[uncategorized] ~211-~211: Loose punctuation mark.
Context: ...lizer applied. * TAN Content: p_nh4_rt (g N / kg) - The amount of t...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~212-~212: Loose punctuation mark.
Context: ...acal form. * Emission Factor: A dimensionless factor representing the...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~233-~233: Loose punctuation mark.
Context: ...(kg / ha). * Total N Content: p_n_rt (fraction). * `Emiss...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~234-~234: Loose punctuation mark.
Context: ...fraction). * Emission Factor: p_ef_nh3 (fraction). This factor can ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~240-~240: Loose punctuation mark.
Context: ..._rt - p_nh4_rt). * p_no3_rt: Nitrate content. * p_nh4_rt`...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~241-~241: Loose punctuation mark.
Context: ... Nitrate content. * p_nh4_rt: Ammonium content (TAN). * `p_...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~242-~242: Loose punctuation mark.
Context: ...nium content (TAN). * p_s_rt: Sulfur content. * K_1, `K_2...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~243-~243: Loose punctuation mark.
Context: ...s_rt: Sulfur content. * K_1, K_2, K_3`: Empirical constants. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~260-~260: Loose punctuation mark.
Context: ...onCatalogue). * Emission Factor`: This factor is calculated based on the ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
fdm-docs/docs/balance/01-nitrogen.md
207-207: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
210-210: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
211-211: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
212-212: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
215-215: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
229-229: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
232-232: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
233-233: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
234-234: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
239-239: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
240-240: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
241-241: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
242-242: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
243-243: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
244-244: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
245-245: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
246-246: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
247-247: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
259-259: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
260-260: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
262-262: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
265-265: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: calculator (22)
- GitHub Check: core (22)
🔇 Additional comments (2)
fdm-docs/docs/balance/01-nitrogen.md (2)
229-234: Verify and document the units forp_n_rt,p_no3_rt,p_nh4_rtEarlier sections define these parameters in g N / kg, but the text here calls them a “fraction”.
Either:
- They should be divided by 1000 in the formula (like the supply calculation), or
- The catalogue now stores them already as fractions – in which case earlier descriptions need updating.
Please confirm which representation is correct and adjust the documentation (and variable descriptions) for consistency.
262-266: Clarify lower bound for residue-based emission factor
Emission Factor = 0.41 × b_lu_n_residue – 5.42becomes negative whenb_lu_n_residue < 13.2 g N / kg.
Negative fractions are physically impossible; indicate that the factor is clamped to ≥ 0 (if that’s how the code behaves) or reference the rule that prevents low-N crops from producing negative values.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fdm-docs/docs/balance/01-nitrogen.md (1)
207-214: Unit-conversion step fixed 👍 — but indentation now violates MD007The
/ 1000conversion has been added (good catch), yet the inner bullet list is indented with 4 spaces instead of the 2-space increment configured bymarkdownlint(see static-analysis warnings on the same lines).
Consider reducing the indent to keep CI/preview clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-docs/docs/balance/01-nitrogen.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
fdm-docs/docs/balance/01-nitrogen.md (1)
Learnt from: SvenVw
PR: SvenVw/fdm#134
File: fdm-calculator/src/balance/nitrogen/index.ts:236-238
Timestamp: 2025-05-26T10:32:15.538Z
Learning: In nitrogen balance calculations for agricultural systems, removal and volatilization are calculated as negative values by definition since they represent nitrogen losses from the system. The balance calculation uses addition (.add()) for all components because removal and volatilization are already negative, so adding them effectively subtracts the losses from the supply.
🪛 LanguageTool
fdm-docs/docs/balance/01-nitrogen.md
[uncategorized] ~211-~211: Loose punctuation mark.
Context: ...lizer applied. * TAN Content: p_nh4_rt (g N / kg) - The amount of t...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~212-~212: Loose punctuation mark.
Context: ...acal form. * Emission Factor: A dimensionless factor representing the...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~234-~234: Loose punctuation mark.
Context: ...(kg / ha). * Total N Content: p_n_rt (fraction). * `Emiss...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~235-~235: Loose punctuation mark.
Context: ...fraction). * Emission Factor: p_ef_nh3 (fraction). This factor can ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~241-~241: Loose punctuation mark.
Context: ..._rt - p_nh4_rt). * p_no3_rt: Nitrate content. * p_nh4_rt`...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~242-~242: Loose punctuation mark.
Context: ... Nitrate content. * p_nh4_rt: Ammonium content (TAN). * `p_...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~243-~243: Loose punctuation mark.
Context: ...nium content (TAN). * p_s_rt: Sulfur content. * K_1, `K_2...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~244-~244: Loose punctuation mark.
Context: ...s_rt: Sulfur content. * K_1, K_2, K_3`: Empirical constants. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~261-~261: Loose punctuation mark.
Context: ...onCatalogue). * Emission Factor`: This factor is calculated based on the ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
fdm-docs/docs/balance/01-nitrogen.md
207-207: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
210-210: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
211-211: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
212-212: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
230-230: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
233-233: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
234-234: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
235-235: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
240-240: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
241-241: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
242-242: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
243-243: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
244-244: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
245-245: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
246-246: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
247-247: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
248-248: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
260-260: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
261-261: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
263-263: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
266-266: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 300000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: core (22)
- GitHub Check: calculator (22)
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores