Allow MA module to accept trait and prior dataframes directly

[AritraDey-Dev]

one should be able to run the MA itself without a full settings object

Originally posted by @mdietze in #3707 (comment)

For more context #3707 (comment) & #3707 (comment)

[Mayanknishad9]

This is a great improvement! My format_try_for_ma() function (PR #3720) creates exactly the kind of trait_data dataframe that this refactored MA module would accept.

Once both are implemented, users could have a clean workflow:

1. try_data <- read.delim("TRY_export.txt")
2. trait_df <- format_try_for_ma(try_data, trait_mapping)
3. run_meta_analysis(trait_df, prior_df, config) This new function from Allow MA module to accept trait and prior dataframes directly #3718

This would make PEcAn's meta-analysis much more accessible to researchers with external data sources like TRY.

[ashiklom]

Isn't this already possible? Here's the signature of the PEcAn.meta.analysis::pecan.ma function.

pecan/modules/meta.analysis/R/meta.analysis.R

Lines 44 to 54 in ad821ea

	##' # Load traits and priors from BETY
	##' species <- PEcAn.DB::query.pft_species(pft, con = con)
	##' trait.data <- PEcAn.DB::query.traits(species[["id"]], c("SLA", "Vcmax"), con = con)
	##' prior.distns <- PEcAn.DB::query.priors(pft_id, trait_string, con = con)
	##'
	##' # Pre-process data
	##' jagged.data <- lapply(trait.data, PEcAn.MA::jagify)
	##' taupriors <- list(tauA = 0.01,
	##' tauB = c(SLA = 1000, Vcmax = 1000))
	##' result <- pecan.ma(jagged.data, prior.distns, taupriors,
	##' j.iter = 5000, outdir = tempdir())

That said, there's a lot of extra logic in run.meta.analysis.pft that could be cleaned up, but my high-level recommendation would probably be to just not use that function and just use PEcAn.meta.analysis::pecan.ma in standalone contexts (like the Quarto notebook cited above).

[ashiklom]

A more general observation: A lot of the stuff in run.meta.analysis.R feels specific to the way full PEcAn workflows are typically executed. As a matter of organization / refactoring, I would suggest moving a lot of that functionality into something like PEcAn.workflow and keep the PEcAn.meta.analysis package specifically focused on executing meta-analysis given trait and PFT information as data frames.

That would also eliminate dependencies on PEcAn.DB, PEcAn.settings, and PEcAn.utils, which would make this much easier to install.

@mdietze -- What do you think?

[mdietze]

Thanks for the suggestions @ashiklom. I suspect you are correct that there would be a lot of advantages to moving the "run module" level into PEcAn.workflow and dropping dependencies on DB, settings, and utils.

I wonder if it would be worth creating a new function within the MA package that would fall between run.meta.analysis.pft and pecan.ma that would keep some of the diagnostics and prior/posterior checks but would leave all the database and RData logic in run.meta.analysis.pft. But I could also be convinced that it would be simpler to just build the MA demo directly around pecan.ma.

[ashiklom]

I wonder if it would be worth creating a new function within the MA package that would fall between run.meta.analysis.pft and pecan.ma that would keep some of the diagnostics and prior/posterior checks but would leave all the database and RData logic in run.meta.analysis.pft

I think this is worthwhile and very doable, as long as everything reliably fits into memory. As long as we're not lugging around large MCMC sampler objects and multiple GB of trait data, it should be fine.

Even then, there are more flexible ways to handle that without forcing a reliance on PEcAn.DB.

[ashiklom]

@mdietze (and others) Here's a quick pass at this: 2d3884c

If that looks OK, I can run the various other necessities (roxygen, etc.) and submit a PR. It's probably safest to merge that first and confirm that everything still works and then to migrate run.meta.analysis.pft and friends into PEcAn.workflow (or wherever we decide) in a separate PR. But if you disagree, I'm happy to tack that on.

[mdietze]

@ashiklom that looks good to me. Two small suggestions

1. pft: I wonder if we can drop this from the function name and make it optional to the function itself. It only seems to be used in the call to pecan.ma.summary, which itself seems to only use it for the purposes of providing written messages in the logs (which are never read 99.9% of the time), which could easily be refactored to be pft-optional with a sensible default.
2. naming: the distinction between run.meta.analysis.pft and run_meta_analysis_pft will not be obvious to users. I wonder if there's a new name that could more obviously make this stand out as the "primary stand alone MA function"

[ashiklom]

Thanks @mdietze! Agreed on both counts.

I wonder if there's a new name that could more obviously make this stand out as the "primary stand alone MA function"

"There are 2 hard problems in computer science: cache invalidation, naming things, and off-by-1 errors." --- Leon Bambrick

[ashiklom]

Closed by #3728.