[Bug fix] : Fixed gsub issue in write.config.SIPNET

[meetagrawal09]

Description

Fixed issues with local deployment for SIPNET model.

The variables cpruncmd and cpoutcmd were added to write.config.SIPNET to extend it for cloud based PEcAn deployment. On a local deployment setup the workflow for SIPNET model was breaking with the below error :

Error in gsub("@CPRUNCMD@", cpruncmd, jobsh) :
  invalid 'replacement' argument
Calls: <Anonymous> ... <Anonymous> -> do.call -> write.config.SIPNET -> gsub
> proc.time()
   user  system elapsed
  7.132   3.362   7.877

The error arises since local pecan.xml does not contain settings$host$rabbitmq$cpfcmd and settings$host$rabbitmq$cpfcmd which is used to set cpruncmd and cpoutcmd in the write.config.SIPNET file.

Review Time Estimate

• Immediately
• Within one week
• When possible

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation.
• My name is in the list of CITATION.cff
• I have updated the CHANGELOG.md.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[mdietze]

Couple questions:

• are we sure that if these variables aren't defined in the pecan.xmxl that they'll be set to character(0)?
• If these variables don't exist, are we sure the job.sh will run correctly with the text of these tags left in the shell script? looking at the template job, it seems like they should be removed (i.e. gsubbed with an empty string)
  

  pecan/models/template/inst/template_geo.job

  Line 46 in 4564337

  @CPRUNCMD@

If the right thing to do is gsub with an empty string, and the is.identical is assuming the default is an empty string, this begs the question of why the default case is failing

[meetagrawal09]

The code segment responsible to set the value of cpruncmd is write.configs.SIPNET.R#L76-L77.

Here since we don't set the value for settings$host$rabbitmq$cpfcmd as part of settings file that means on addressing this variable we get NULL. gsub returns a character(0) value when the target string is NULL.

Basically the line transforms to :

cpruncmd <- gsub("@OUTDIR@", settings$host$rundir, NULL)

which is why a check for this is needed in place.

[infotroph]

@meetagrawal09 I agree with your diagnosis, but I think the issue should be fixed on lines 76-77 rather than here. Maybe that could be something like cpruncmd <- gsub("@OUTDIR@", settings$host$rundir, settings$host$rabbitmq$cpfcmd %||% ""), then the replacements on lines 105-106 remain unconditional?

[mdietze]

@meetagrawal09 this syntax doesn't work in R. First, if(settings$host$rabbitmq$cpfcmd == NULL) throws an "argument is of length zero" error if the variable is NULL and should be replaced with is.null(). Second, the if ... else syntax should be replaced with the ifelse() function.

[infotroph]

Or the null-coalescing operator %||% as I showed above—it’s made for exactly this case

[meetagrawal09]

Don't know but this operator did not work for me. Shows me the below error.

Error in settings$host$rabbitmq$cpfcmd %||% "" :  could not find function "%||%"

[infotroph]

Oh, hm.
(1) You'll need to import it[1] from rlang with #' @importFrom rlang %||% or just copy in the definition: function (x, y) { if (is.null(x)) y else x} ...
(2) ...which reminds me that %||% only works on scalars. @mdietze does your point about ifelse mean cpfcmd might have legnth >1? If so my advice was bad to start with and ifelse seems cleaner.

[1] (%||% has been added to base R in 4.4.0, so a few version bumps from now we'll be able to use it with no import)