pecan
pecan copied to clipboard
Fixed Global function and variables from PEcAn.emulator
@mdietze In case when no arguments passed for eg in line 105 of modules/emulator/R/predict.GP.R should we use utils::setTxtProgressBar() directly? as we can not determine the min and max value
@nanu1605 1) if we switch to a new function we need to switch to that new functions args/usage, we can't just keep using it like the old function, nor can we use the function for updating the progress bar without defining it 2) in this case we most definitely do know the min, max, and state -- if you look on line 156 you see that the counter goes up to max = length(samp) and the counter variable is i instead of g
@infotroph I think changing pda.calc.llik.par to PEcAn.assim.batch::pda.calc.llik.par is creating a circular dependency between emulator and assim.batch. Should we move the function to some other package?
Poking around a bit I think the best option to remove the circular dependency would be to move the functions in minimize.GP.R that use pda.calc.llik* into assim.batch, but I haven't fully checked yet whether that would just create a different circular dependency in the other direction if those functions are used elsewhere in emulator.
@mdietze I think we can move the whole file (minimize.GP.R) to assim.batch as all the functions in the minimize.GP.R are only used in assim.batch.
@nanu1605 I resolved the conflicts from merging #3027 and added a few more cleanup items to get checks passing -- can you please review my commits from today?
I also left some suggestions above on the txtProgressBar calls -- I think they should be initialized once before the loop and then set inside it, not reinitialized.
Sure @infotroph, I will do it ASAP
@mdietze I think we've addressed your change requests and this is ready for re-review