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

Jump to bottom

Refactor met.process and dbfiles #3319

Merged
merged 33 commits into from
Sep 21, 2024
Merged

Refactor met.process and dbfiles #3319

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f9a0cff
Refactor met.process.R to correctly handle standerdized_result
Sweetdevil144 Jun 26, 2024
dd53e34
Remove ucommented sections and replace NULL declarations with c()
Sweetdevil144 Jun 27, 2024
2a9e286
Revert to default NULL instead of c()
Sweetdevil144 Jun 28, 2024
45a7019
Merge branch 'PecanProject:develop' into convert-input
Sweetdevil144 Jun 28, 2024
7214403
Add small modification to dbfiles.R
Sweetdevil144 Jun 28, 2024
a61d38e
Fix 'return' mistake in dbfiles.R
Sweetdevil144 Jun 28, 2024
3509396
Merge branch 'PecanProject:develop' into convert-input
Sweetdevil144 Jul 9, 2024
da6955a
Omit return Type
Sweetdevil144 Jul 10, 2024
36b661a
Merge branch 'develop' into convert-input
Sweetdevil144 Jul 11, 2024
3eddf58
Update return statement in dbfiles.R
Sweetdevil144 Jul 11, 2024
3314842
Merge branch 'PecanProject:develop' into convert-input
Sweetdevil144 Jul 13, 2024
afaf73f
Update invisible statements to `return` after execution
Sweetdevil144 Jul 13, 2024
0b3d0c4
refactor return statements
Sweetdevil144 Jul 13, 2024
e36396f
Merge branch 'develop' into convert-input
Sweetdevil144 Jul 13, 2024
978309a
Merge branch 'develop' into convert-input
Sweetdevil144 Jul 18, 2024
604e03f
Merge branch 'develop' into convert-input
Sweetdevil144 Jul 25, 2024
f665ed8
Merge branch 'PecanProject:develop' into convert-input
Sweetdevil144 Jul 31, 2024
d33631c
Merge branch 'develop' into convert-input
Sweetdevil144 Jul 31, 2024
cbfb7c1
Merge branch 'develop' into convert-input
Sweetdevil144 Aug 5, 2024
9f9dc73
Merge branch 'develop' into convert-input
Sweetdevil144 Aug 9, 2024
23c7809
Merge branch 'develop' into convert-input
Sweetdevil144 Aug 12, 2024
4ffd0db
Merge branch 'develop' into convert-input
Sweetdevil144 Aug 14, 2024
c276f78
Merge branch 'develop' into convert-input
Sweetdevil144 Aug 16, 2024
383443c
Merge branch 'develop' into convert-input
Sweetdevil144 Aug 28, 2024
4f9320e
Update base/db/R/dbfiles.R
Sweetdevil144 Aug 30, 2024
e9367b5
Apply suggestions from code review
Sweetdevil144 Aug 30, 2024
3ff0eb3
Apply standardization changes
Sweetdevil144 Aug 30, 2024
7ff5dce
Merge branch 'develop' into convert-input
Sweetdevil144 Aug 31, 2024
60be255
Merge branch 'develop' into convert-input
Sweetdevil144 Sep 3, 2024
49502b9
Merge branch 'develop' into convert-input
Sweetdevil144 Sep 5, 2024
8405afb
Merge branch 'develop' into convert-input
Sweetdevil144 Sep 12, 2024
665a5ec
Merge branch 'develop' into convert-input
Sweetdevil144 Sep 15, 2024
4d37521
Merge branch 'develop' into convert-input
infotroph Sep 21, 2024
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
8 changes: 4 additions & 4 deletions base/db/R/dbfiles.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,13 @@ dbfile.input.check <- function(siteid, startdate = NULL, enddate = NULL, mimetyp
formatid <- get.id(table = "formats", colnames = c("mimetype_id", "name"), values = c(mimetypeid, formatname), con = con)

if (is.null(formatid)) {
invisible(data.frame())
return(invisible(data.frame()))
Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved
}

# setup parent part of query if specified
if (is.na(parentid)) {
parent <- ""
} else {
parent <- ""

if (!is.na(parentid)) {
parent <- paste0(" AND parent_id=", parentid)
}
Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
19 changes: 12 additions & 7 deletions modules/data.atmosphere/R/met.process.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,9 @@ met.process <- function(site, input_met, start_date, end_date, model,
# Change to Site Level - Standardized Met (i.e. ready for conversion to model specific format)
if (stage$standardize) {
standardize_result <- list()

is_standardized <- FALSE
ready.id <- list(input.id = NULL, dbfile.id = NULL)

Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved
for (i in seq_along(cf.id[[1]])) {

if (register$scale == "regional") {
Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -301,6 +303,7 @@ met.process <- function(site, input_met, start_date, end_date, model,
host = host,
overwrite = overwrite$standardize)
# Expand to support ensemble names in the future
is_standardized <- TRUE
Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved
} else if (register$scale == "site") {
##### Site Level Processing
standardize_result[[i]] <- .metgapfill.module(cf.id = list(input.id = cf.id$input.id[i], dbfile.id = cf.id$dbfile.id[i]),
Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -314,15 +317,17 @@ met.process <- function(site, input_met, start_date, end_date, model,
host = host,
overwrite = overwrite$standardize,
ensemble_name = i)
is_standardized <- TRUE
} else {
is_standardized <- FALSE
Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved
}

if(is_standardized) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this changes behavior: previously ready.id would have an entry for each id whether it was standardized or not; this only adds the ids that were standardized. Is that intentional?

(As currently implemented I think ids not standardized would probably have errored out earlier, but this step should not rely on that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've implemented a NA appending too to make sure to append even when we have face a !is_standardized condition. The initial though process I had was whether it would be wise pass the input.id's to .met2model.module and later on to convert_input if we have not standardised our results. Although it seems I failed and ignored the order in array at check which may change the results.

Copy link
Contributor Author

@Sweetdevil144 Sweetdevil144 Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@infotroph Can you take a look into this?

Copy link
Member

Choose a reason for hiding this comment

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

@Sweetdevil144 I was wrong above about the existing behavior -- In the current code standardize_result has an entry for each id, but it is NULL for ids not standardized and therefore gets silently dropped when appending to ready.id. I don't know how advisable that is, but I bet downstream code relies on it.

I'll suggest changes here that eliminate the second for-loop but keep the original behavior -- please edit further if you see problems I missed.

ready.id$input.id <- c(ready.id$input.id, standardize_result[[i]]$input.id)
ready.id$dbfile.id <- c(ready.id$dbfile.id, standardize_result[[i]]$dbfile.id)
}

} # End for loop
ready.id <- list(input.id = NULL, dbfile.id = NULL)

for (i in seq_along(standardize_result)) {
ready.id$input.id <- c(ready.id$input.id, standardize_result[[i]]$input.id)
ready.id$dbfile.id <- c(ready.id$dbfile.id, standardize_result[[i]]$dbfile.id)
}

} else {
ready.id <- input_met$id
Expand Down
Loading