-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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"
# Conflicts: # DESCRIPTION # NEWS.md
- 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
Codecov Report
@@ 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
|
- 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
N1ckP3rsl3y
approved these changes
Aug 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
previously, this was targeted for v6.0.1 (PR Development towards v6.0.1 #235)