Update write.configs.FATES.R #3360
Conversation
There was a problem hiding this comment.
There's a lot that confuses me in this PR. First, it looks like this code is behind the mainline as it's deleted a lot of other people's changes. Second, it's deleted a ton of the FATES coupler without a clear substitution of an alternative structure for solving the same problem.
| @@ -120,9 +120,7 @@ authors: | |||
| - given-names: Eric R. Scott | |||
| affiliation: University of Arizona | |||
| orcid: 'https://orcid.org/0000-0002-7430-7879' | |||
| - given-names: Harunobu Ishii | |||
There was a problem hiding this comment.
Unclear why you need to remove someone else's contribution
| @@ -2,6 +2,8 @@ | |||
| # | |||
| # docker-compose -f docker-compose.yml -f docker-compose.dev.yml | |||
|
|
|||
| version: '3.2' | |||
There was a problem hiding this comment.
does this PR really need to touch docker files?
|
|
||
| trait.values$temperate.coniferous <- tc_traits | ||
| settings <- '/pecan/models/fates/R/test.fates.xml' | ||
| run.id <- 'test_oneyear' |
There was a problem hiding this comment.
Was this supposed to be an example? You can't have code like this that's outside of function.
| write.config.FATES <- function(defaults, trait.values, settings, run.id){ | ||
|
|
||
| ## site information | ||
| site <- settings$run$site | ||
| site.id <- as.numeric(site$id) | ||
| site.id <- site$id # change1: 772 -> SOD1 |
There was a problem hiding this comment.
not following the logic of the change here. Comment is also unclear to a general user
| default <- settings$run$inputs$default$path ## reference inputs file structure | ||
| site_name <- paste0(site.id %/% 1000000000, "-", site.id %% 1000000000) | ||
| site_name <- paste0(site.id, "_c", run.id) ## change3 |
There was a problem hiding this comment.
I'm a bit concerned about the logic change here and on the casedir line. Could you explain what you're intending? Be aware that changes of internal logic can't change the nature of what PEcAn is passing into each function.
| # } | ||
| # if (!is.null(settings$host$postrun)) { | ||
| # hostteardown <- paste(hostteardown, sep="\n", paste(settings$host$postrun, collapse="\n")) | ||
| # } |
There was a problem hiding this comment.
For all these large chunks of code that you've commented out, are these things that are no longer used by CLM-FATES?
| for (v in seq_along(pft)) { | ||
| var <- names(pft)[v] | ||
| #for (v in seq_along(pft)) { | ||
| #var <- names(pft)[v] |
There was a problem hiding this comment.
Similar to previous comment, what's the reason for commenting out the last ~50 lines?
| if(var == "Vcmax"){ | ||
| ncdf4::ncvar_put(nc=fates.param.nc, varid='fates_vcmax25top', start = ipft, count = 1, | ||
| vals=pft[v]) ## (umol CO2 m-2 s-1) | ||
| } |
There was a problem hiding this comment.
have all of these variable names changed or are you pursuing a conceptually different approach for mapping PEcAn/BETY parameters to fates parameters?
| ncdf4::nc_close(clm.param.nc) | ||
| ncdf4::nc_close(fates.param.nc) | ||
| #ncdf4::nc_close(clm.param.nc) | ||
| #ncdf4::nc_close(fates.param.nc) |
There was a problem hiding this comment.
got to the end of this and do see where a new alternative way of parameter mapping ended up being implemented
| @@ -32,7 +32,6 @@ Imports: | |||
| stringr | |||
| Suggests: | |||
| corrplot, | |||
| exactextractr, | |||
There was a problem hiding this comment.
Why does this PR require changes to assim.sequential?
|
Thanks for your detailed comment! @mdietze This PR is to show the steps in updating write.config file, and currently it's on the first stage to run the basic workflow of fates, where many fates coupler is uncommented for clarity and would be completely updated in the end. One problem is since my docker got stuck to open BETY webpage, temporarily some local address is used and name in the script is changed to run the fates model, just as you mentioned in the above comments. I would be grateful if you could have any idea in helping me solve the issue related to BETY. As for the deleted contributions, I'm also confused cause I pushed my changes after having pulled the develop branch from upstream pecan. But I will check the version of my repository, and see if there are some problems to be solved. |
Description
To update write.configs file in fates model, the goal is divided into three stages:
Motivation and Context
The first PR aims to generate basic template.job to run fates model in pecan workflow. However, the web interface of Bety doesn't work on the local machine. Thus, address of local dataset was used.
Review Time Estimate
Types of changes
Checklist: