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.

Sign up for GitHub

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

set defaults for new ED2IN vars #3033

Merged
merged 14 commits into from
Sep 28, 2022
Merged

set defaults for new ED2IN vars #3033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
03b6e24
set defaults for new vars
Aariq Sep 13, 2022
808a7b7
Merge branch 'develop' into ed2in-defaults
dlebauer Sep 14, 2022
152de62
Merged upstream/develop into ed2in-defaults
Aariq Sep 15, 2022
b65a910
add test that last line of ED2IN is $END
Aariq Sep 15, 2022
f1e9793
fix 3035 by moving the $END line to after the new tags
Aariq Sep 15, 2022
4783fb9
upate changelog
Aariq Sep 15, 2022
dcf8d6b
Apparently the !=== lines are important
Aariq Sep 15, 2022
ba346ed
$END is before !=== in templates
Aariq Sep 15, 2022
e543bf6
Merge branch 'develop' into ed2in-defaults
Aariq Sep 15, 2022
e5264ec
Merge branch 'develop' into ed2in-defaults
Aariq Sep 19, 2022
0acd793
Merge branch 'develop' into ed2in-defaults
dlebauer Sep 20, 2022
99ac34e
Merge branch 'develop' into ed2in-defaults
dlebauer Sep 20, 2022
f2b171a
Merge branch 'develop' into ed2in-defaults
Aariq Sep 26, 2022
b4ddf91
Merge branch 'develop' into ed2in-defaults
Aariq Sep 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ convert data for a single PFT fixed (#1329, #2974, #2981)
Note that both `units` and `udunits2` interface with the same underlying
compiled code, so the `udunits2` *system library* is still required.
(#2989; @nanu1605)
- Fixed a bug with ED2 where ED2IN tags supplied in `settings` that were not in the ED2IN template file were not getting added to ED2IN config files (#3034)
- Fixed a bug with ED2 where ED2IN tags supplied in `settings` that were not in the ED2IN template file were not getting added to ED2IN config files (#3034, #3033)
- Fixed a bug where warnings were printed for file paths on remote servers even when they did exist (#3020)
- Added an updated ED2IN template file, `models/ed/inst/ED2IN.r2.2.0.github`, to reflect new variables in the development version of ED2
- `PEcAn.data.land::gSSURGO.Query` has been updated to work again after changes to the gSSURGO API.

### Changed
Expand Down
24 changes: 19 additions & 5 deletions models/ed/R/write_ed2in.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,19 @@ write_ed2in.ed2in <- function(ed2in, filename, custom_header = character(), bare
nvalues <- length(tags_values_vec)
ncomments <- length(attr(ed2in, "comment_values"))
file_body <- character(nvalues + ncomments)
file_body[attr(ed2in, "comment_linenos")] <- attr(ed2in, "comment_values")
file_body[attr(ed2in, "value_linenos")] <- tags_values_vec[1:length(attr(ed2in, "value_linenos"))]
file_body[attr(ed2in, "comment_linenos")] <-
attr(ed2in, "comment_values")
file_body[attr(ed2in, "value_linenos")] <-
tags_values_vec[1:length(attr(ed2in, "value_linenos"))]

#check for new tags
if(length(tags_values_vec) > length(attr(ed2in, "value_linenos"))) {
new_tags <- tags_values_vec[(length(attr(ed2in, "value_linenos")) + 1):length(tags_values_vec)]
file_body <- c(file_body, new_tags)
#find the $END
END_line <- grep("\\$END", file_body) - 1
new_tags <-
tags_values_vec[(length(attr(ed2in, "value_linenos")) + 1):length(tags_values_vec)]
#put the new tags in with $END at the end
file_body <- c(file_body[1:END_line], new_tags, "$END")
}
header <- c(
"!=======================================",
Expand Down Expand Up @@ -60,7 +66,15 @@ write_ed2in.default <- function(ed2in, filename, custom_header = character(), ba
paste0("! ", custom_header),
"!---------------------------------------"
)
output_lines <- c(header, "$ED_NL", tags_values_vec, "$END")
output_lines <-
c(
header,
"$ED_NL",
tags_values_vec,
"$END",
"!==========================================================================================!",
"!==========================================================================================!"
)
writeLines(output_lines, filename)
}

Expand Down
10 changes: 6 additions & 4 deletions models/ed/inst/ED2IN.r2.2.0.github
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,11 +599,13 @@ $ED_NL
! They are used only when ISOILFLG is 2, both values are between 0. and 1., and !
! their sum doesn't exceed 1. In case ISOILFLG is 2 but the fractions do not meet the !
! criteria, ED-2 uses NSLCON instead. !
! NOTE: these defaults are arbitrary, but I don't think that matters since !
! SOIL_HYDRO_SCHEME=0 !
!---------------------------------------------------------------------------------------!
NL%SLSOC = myslsoc
NL%SLPH = myslph
NL%SLCEC = myslcec
NL%SLDBD = mysldbd
NL%SLSOC = 0.06
NL%SLPH = 6
NL%SLCEC = 0.5
NL%SLDBD = 1330
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand why you switch out variables for hard-coded constants. Doesn't this eliminate the ability for PEcAn to set these variables?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are new variables that didn't exist in previous versions of ED2IN. I assumed I would need knowledge of soil chemistry to connect these up to PEcAn in some way, but if that's not the case I can take a look at how other ED2IN variables are done and take a crack at this. Also, I'm pretty sure they aren't used unless another hard-coded variable is changed.

Copy link
Member

Choose a reason for hiding this comment

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

So if the mysl* variables don't exist anywhere else in the code I'm OK with this PR, though I do think the soil.nc files do have most of these variables, if one wanted to set up the ability to specify these via code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll open an issue as a reminder to add this in the future

!---------------------------------------------------------------------------------------!


Expand Down
9 changes: 8 additions & 1 deletion models/ed/tests/testthat/test.write.configs.ed.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,14 @@ test_that("New ED2IN tags get added at bottom of file", {

#check that info is printed
expect_true(any(stringr::str_detect(x, "NEW_TAG")))


#check that last non-comment line of ED2IN is "$END"
#TODO someone better at regex could do this more efficiently
lines <- trimws(readLines(file.path(rundir, run.id, "ED2IN")))
not_comments <- lines[stringr::str_detect(lines, "^!", negate = TRUE)]
not_spaces <- not_comments[stringr::str_detect(not_comments, ".+")]
expect_equal(not_spaces[length(not_spaces)], "$END")

#6. compare to template
# ed2in_template <- read_ed2in(system.file(settings$model$edin, package = "PEcAn.ED2"))
# Not sure what to expect regarding tag names or number of tags relative to template
Expand Down