write.config.SIPNET warns about multiple vegetation PFTs when there's only one
Maintainers usually reply within 1 day
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 72/100
Research direction
Read models/sipnet/R/write.configs.SIPNET.R around lines 94–102 and compare how the ed and dalec couplers handle env. Then inspect test-write.config.SIPNET.R and add coverage for one vegetation PFT with empty env and for two real vegetation PFTs. Done when the empty env case produces no warning while two vegetation PFTs still trigger it; run the SIPNET tests.
Written by the indexing model from the issue text.
Description
Running Demo 1 (Basic Run) on develop (22ab283, macOS arm64, R 4.5.2), I get this warning once per ensemble member, so 10 times:
WARN [write.config.SIPNET] : SIPNET can only handle one vegetation PFT and one optional soil PFT. Passing multiple vegetation PFTs may lead to unexpected behavior.
The demo only has one PFT (temperate.coniferous), so I looked for where the second one comes from. The ensemble sampler always adds an env entry to the samples, even when it's empty:
So write.config.SIPNET receives trait.values with two names, temperate.coniferous and env. The check counts everything that isn't soil, so env looks like a second vegetation PFT:
Other couplers already skip env (for example write.configs.ed.R line 494 and write.configs.dalec.R line 87), so I think SIPNET's check just needs to do the same. Unless counting it is intentional?
What I'd suggest: leave soil and env out of the count before checking.
n_veg_pfts <- length(trait.values) - sum(names(trait.values) %in% c("soil", "env"))
if (n_veg_pfts > 1) {
# existing warning
}
Plus a test in test-write.config.SIPNET.R: one PFT with an empty env shouldn't warn, and two real PFTs still should. I have this ready locally: the new test fails on develop and passes with the change, the rest of the SIPNET tests still pass, and Demo 1 goes from 10 warnings to 0.
I didn't find an existing issue for this. Happy to open a PR if this sounds right.
Disclosure: I used Claude Code (an AI coding agent) to run the demo on my machine, trace the cause, test the fix and draft this note.
@infotroph @divine7022
- Dominant language
- R
- Stars
- 246
- Forks
- 322
- Avg merge
- 4d 22h
- Merged PRs (30d)
- 19
Getting set up
- Ships a Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from PecanProject/pecan
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
PecanProject/pecan#4236 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
PecanProject/pecan#4235 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
PecanProject/pecan#4234 · 2 comments ·
Maintainers usually reply within 1 day
-
download.SM_CDS: CDS rejects the request, then the retry loop never endsPossibly taken @S1DDHEY claimed this 3 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
PecanProject/pecan#4219 · 3 comments · 1 assignee ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 1-3 hours Newbie friendliness 82/100
PecanProject/pecan#4218 · 1 comment ·
Maintainers usually reply within 1 day
All issues in PecanProject/pecan
Similar issues
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 80/100
-
Difficulty 1/5 Under an hour Newbie friendliness 95/100
OHDSI/CohortGenerator#281 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100