Snt25 378

[claude-marie]

To sum up what was asked in that ticket:
https://bluesquare.atlassian.net/browse/SNT25-378

1. add a USE_CALENDAR_YEAR_DENOMINATOR parameter
2. move compute_month_seasonality_NEW and compute_month_seasonality in snt_utils.r
3. update rainfall & cases to use these
4. clarification & comments in the notebook
5. new description for the pipeline using Giulia prompt (I'm still updating it in openhexa bc it is ugly as hell atm)
6. some fixes in the reporting notebooks

[sPuntinG]

I see the point of adding a safety guard, but I don't think it's necessary to re-define the function here ... it adds too much clutter.
I'd issue a clear message instead ("Error: the function xxx does not exist! Enure that the snt_utils.r file is updated to latest version" or similar).

Also, @EstebanMontandon what is the logic for updating the snt_utils.r file?
Do we have a best practice to update this file in an already existing workspace?

[EstebanMontandon]

Currently we maintain the latest version of the snt_utils.r module in the repository, so every time we select the options "pull scripts" when executing a pipeline, this file gets updated (replacing the file currently in that workspace).
We haven't defined a specific best practice for managing updates to this file, but since all pipelines depend on the code defined here, standard good practices apply: changes must always be backward compatible to avoid breaking existing pipeline logic.

[EstebanMontandon]

btw, I think @sPuntinG is correct in this point, we don't need to do extra validation when calling our own functions, these supposed to be designed so serve specific SNT computations, so we can safely ignore it

[sPuntinG]

This should go as it looks like it overrides the pipeline parameters, no?

It should have something like this (this is what I added in the rainfall rerpoting nb in NER):

# Parameters - Load from dataset if available, otherwise use defaults
parameters_file <- paste0(COUNTRY_CODE, "_parameters.json")
seasonality_dataset <- config_json$SNT_DATASET_IDENTIFIERS$SNT_SEASONALITY_RAINFALL

# Try to load parameters from dataset
parameters <- tryCatch({
    # Get parameters JSON file from dataset (returns a list for JSON files)
    get_latest_dataset_file_in_memory(seasonality_dataset, parameters_file)
}, error = function(e) {
    log_msg(paste0("[WARNING] Could not load parameters from dataset, using defaults: ", conditionMessage(e)), level = "warning")
    NULL
})

# Set parameters from dataset or use defaults
if (!is.null(parameters) && is.list(parameters)) {
    minimum_month_block_size <- as.integer(parameters$minimum_month_block_size)
    maximum_month_block_size <- as.integer(parameters$maximum_month_block_size)
    threshold_for_seasonality <- as.numeric(parameters$threshold_for_seasonality)
    threshold_proportion_seasonal_years <- as.numeric(parameters$threshold_proportion_seasonal_years)
    log_msg(paste0("Parameters loaded from dataset: ", parameters_file))
} else {
    # Default values
    minimum_month_block_size <- as.integer(3)
    maximum_month_block_size <- as.integer(5)
    threshold_for_seasonality <- 0.6
    threshold_proportion_seasonal_years <- 0.5
    log_msg("Using default parameter values")
}

[sPuntinG]

Same as in CASES seasonality: I don't think that we need to define the function here again (just issue a descriptive message prompting to update the snt_utils.r file)

[sPuntinG]

Just a question/curiosity: this is your new approach for identifying the starting month @claude-marie ?

[claude-marie]

Yes this is, I can tell you about on a quick call if you want :)

[sPuntinG]

Noice job!
But please check my comments and implement the suggested changes (if they makes sense to you too) before merging :)

[sPuntinG]

Also @claude-marie on this:

5. new description for the pipeline using Giulia prompt (I'm still updating it in openhexa bc it is ugly as hell atm)

do you also find the MD editor in the OH pipelines description a bit annoying to work with?
(that is also way I was suggesting to put the link to the GitHub pipeline readme.md for the better looking formatted description ... !)

[claude-marie]

@sPuntinG YES, the description part is super annoying. It doesn't understand markdown and you have to do everything yourself which is super annoying. It would be great to be able to drop a MD

[sPuntinG]

Looks all good :)

I just couldn't make sense of the merge conflict because of the ipynb structure that is really painful to diff ... so I trust you'll handle that 😅