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

Prepare development for v6.0.2 #238

Merged
merged 59 commits into from
Aug 4, 2023
Merged

Prepare development for v6.0.2 #238

merged 59 commits into from
Aug 4, 2023

Conversation

dschlaep
Copy link
Member

Accommodate SOILWAT2 v7.1.0 developments (DrylandEcology/SOILWAT2#351)

  • Updating rSOILWAT2 to be compatible with SOILWAT2 v7.1.0 (thread-safety and reentrancy) will not affect the user interface and is expected to produce identical simulation output

    • -> for rSOILWAT2, this is a patch and not a minor version increase
  • previously, this was targeted for v6.0.1 (PR Development towards v6.0.1 #235)

dschlaep and others added 30 commits June 9, 2023 14:44
- SOILWAT2 has gained a comprehensive structure that controls almost all other structs in the program (mainly dealing with simulation values)
- SOILWAT2 also has other structs dealing with files (LOG_INFO), paths (PATH_INFO), etc.
	* `SW_R_lib.c/h` gain these variables as globals (SW_ALL, PATH_INFO, LOG_INFO, SW_OUTPUT_POINTERS)
- `SW_R_lib.c` also gains "EchoInits" and "QuietMode" as globals since on the SOILWAT2 side, they have been converted from a global to a local variable in `main()`

- See DrylandEcology/SOILWAT2#349 for more information
- Previously, SW_SITE contained a variable called "lyr" which held a dynamic amount of informations for up to a MAX_LAYERS amount of layers
	* This has been removed in the newest development of SOILWAT2 to control less dynamically allocated memory and all variables have been moved to SW_SITE
	* Removed any instance of "lyr" being used and replaced with direct access through SW_SITE
- On the SOILWAT2 side, variable type LOG_INFO holds information pertaining to a log file
- Also on the SOILWAT2 side, `LogError()` was updated to no longer use a log file pointer directly, but now takes in LOG_INFO as an argument
- A lot of SOILWAT2 functions had their interfaces updated as a part of the program becoming threadsafe
	* This commit directly updates function calls that were not included in previous commits dealing with individual types (SW_WEATHER, SW_MODEL, etc.)
- Previously, "EchoInits" was made local to "SW_R_lib.c" but is needed in more than one file
	* `SW_R_lib.h` gains "EchoInits" as an externed global
- The function `SW_F_name()` was removed as a result of functions gaining access to LOG_INFO
	* Instead of calling a function only to return a string, it was more direct and explanatory to use "InFiles" within PATH_INFO
- SOILWAT2 completely removed the global "errstr", so any function in need of it, gains a local version of it
SW_R_lib.c
	- Mainly, functions in need of LOG_INFO now have the global instance sent in
	- Corrected "InFiles" when being sent in to `sw_init_args()`

rSW_*.c
	- Calls to `Str_Dup()` are now sent in the global instance of LOG_INFO
	- Update any other calls to SOILWAT2 functions that have had their interfaces changed
	- Update use of macro `ForEachSoilLayer()` in rSW_SoilWater.c to take in SW_SITE's "n_layers
- SOILWAT2 removed "_Sep" as a global, so the function `onSet_SW_OUT()` can no longer access it
- `onGet_SW_OUT()` gains it as a local variable set to a tab (\t)
- Previous commit: 12ac215
- Fully convert SW_GEN_OUT in rSW_Output.c
	* Functions gain pointer to global SW_GEN_OUT (within SW_ALL) to simplify/reduce the accessing of the variables
- Finish the conversions started in the previous commit: 148260a
- In the previous commit (5a02f5a), calls to `interpolate_monthlyValues()` and `calcSiteClimate()` used "SW_Model" which wasn't committed
- SOILWAT2, like `SW_F_name()`, removed `SW_WeatherPrefix()` because it was found calling another function was superfluous
- Remove "// externs ..." comments
- Include SW_Site.h for enums "N_SWRCs" and "N_PTFs" and the functions `swrc2str()` and `ptf2str()`
- Correct capitalization of "LogInfo"
Nicholas Persley and others added 12 commits June 22, 2023 16:31
- SOILWAT2 moved all instances of LOG_INFO in function prototypes to the last position aside from `LogError()`
- This function was updated on the SOILWAT2 side because of the lack of necessity for the type PATH_INFO
- lintr v3.1.0 https://github.com/r-lib/lintr/releases/tag/v3.1.0

- applied lints
* [sort_linter] sort(years, na.last = TRUE) is better than years[order(years)]
* [matrix_apply_linter] Use rowSums(...) rather than apply(..., MARGIN = 1, FUN = sum)
* [boolean_arithmetic_linter] Use any() to express logical aggregations. For example, replace length(which(x == y)) == 0 with !any(x == y).
* unnecessary_nested_if_linter] Don't use nested `if` statements, where a single `if` with the combined conditional expression will do. For example, instead of `if (x) { if (y) { ... }}`, use `if (x && y) { ... }`
* [unnecessary_lambda_linter] Pass pdf directly as a symbol to lapply() instead of wrapping it in an unnecessary anonymous function. For example, prefer lapply(DF, sum) to lapply(DF, function(x) sum(x)).
* [implicit_assignment_linter] Avoid implicit assignments in function calls. For example, instead of `if (x <- 1L) { ... }`, write `x <- 1L; if (x) { ... }`.
--> however, I believe that those in tests are false positives for those tests that test assignments via a replacement method, etc. --> silence `implicit_assignment_linter`

- newly silenced linters
* indentation_linter
* implicit_assignment_linter (because of perceived false positives in test code)
- It's possible for a SOILWAT2 to crash when being called from an R function
- When that happens, memory could not be properly deallocated and thus cause a memory leak
- Using a local variable, sw_exec, sw_inputDataFromFiles, dbW_generateWeather, and sw_outputData detect if C quit early and frees all model memory
@dschlaep dschlaep marked this pull request as draft July 27, 2023 13:20
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #238 (3ed51b7) into main (22e6011) will increase coverage by 0.08%.
The diff coverage is 83.58%.

❗ Current head 3ed51b7 differs from pull request most recent head e19e793. Consider uploading reports for the commit e19e793 to get more accurate results

@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
+ Coverage   58.61%   58.69%   +0.08%     
==========================================
  Files          42       42              
  Lines        8058     8074      +16     
==========================================
+ Hits         4723     4739      +16     
  Misses       3335     3335              
Files Changed Coverage Δ
R/Rsw.R 68.85% <ø> (ø)
src/rSW_VegEstab.c 46.01% <29.41%> (ø)
src/rSW_Model.c 73.01% <33.33%> (ø)
src/rSW_SoilWater.c 45.45% <50.00%> (ø)
src/rSW_Markov.c 35.22% <60.00%> (ø)
src/rSW_Carbon.c 96.00% <88.88%> (ø)
src/rSW_VegProd.c 99.80% <90.00%> (ø)
src/rSW_Site.c 97.73% <95.74%> (ø)
src/rSW_Output.c 97.54% <97.77%> (+0.12%) ⬆️
R/swWeatherGenerator.R 36.26% <100.00%> (-0.14%) ⬇️
... and 5 more

Nicholas Persley and others added 6 commits July 28, 2023 20:19
- This method of memory freeing does not work, so it has been undone
* Updates to match SOILWAT2's reentrancy and threadsafety preparations
     * Add SOILWAT2's new main types - SW_ALL, SW_OUTPUT_POINTERS, LOG_INFO, and PATH_INFO - as globals
* SOILWAT2 pull request: DrylandEcology/SOILWAT2#349
@dschlaep dschlaep requested a review from N1ckP3rsl3y August 4, 2023 17:38
@dschlaep dschlaep marked this pull request as ready for review August 4, 2023 21:05
@dschlaep dschlaep merged commit e436a76 into main Aug 4, 2023
@dschlaep dschlaep deleted the release/devel_v6.0.2 branch August 4, 2023 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants