-
Notifications
You must be signed in to change notification settings - Fork 235
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.
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
Added new Rmd template for basic PEcAn workflow to PEcAn.all package #1733
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super useful! Need the do.conversions and Traits step.
```{r read.settings} | ||
settings <- PEcAn.settings::read.settings("pecan.xml") | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do conversions:
settings <- PEcAn.utils::do_conversions(settings)
Traits:
settings <- PEcAn.DB::runModule.get.trait.data(settings)
PEcAn.settings::write.settings(settings, outputfile='pecan.TRAIT.xml')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss this at next week's meeting. We had previously worked hard to migrate from having 3 different versions of workflow.R to having having a single canonical version because having multiple workflows was causing a ton of problems both for development (having to implement the same thing in multiple places) and debugging (failing to implement the same thing in multiple places). The fact that @tonygardella already identified such a discrepancy within the originating PR is pretty indicative that the proposed addition will revert us to that condition again.
@mdietze I thought about discussing first but wanted to put this up there as a starting point. Perhaps if we really make this bare-bones and/or automatically parse lines from workflow.R during one of the make steps, we could keep them consistent. But I think that having an Rmd template is a fundamentally different use case than having an automated R script. The scripts/workflow*.R files are not user friendly for interactively debuging a workflow, or for keeping track of science methods and rationale , notes etc. and ultimately publishing a workflow as a reproducible(ish?) analysis. The status and if/else get in the way. PS we currently have four workflow scripts ... |
Merge remote-tracking branch 'origin' into rmd_template # Conflicts: # CHANGELOG.md
@dlebauer I'll bring this up next week in the weekly discussion. I ultimately think we just need a page in documentation outlining how to move to using pecan through Rstudio/R. |
Without prejudice to the good points above about whether we want multiple workflows, a suggestion: If we do want this template, Would it make sense for it to live in |
@dlebauer @infotroph @mdietze I'm going to close this and open this as an issue for discussion. I like the idea of having a template to make it easy for people to run pecan outside of the web interface and perhaps direct people on where people can customize their runs. However, I don't like the idea of having to maintain multiple workflow scripts. I personally think we should have this live in documentation as a page describing and breaking down workflow.R so people can do a basic run from the command line or from within R studio. |
I am re-opening because I don't think that the underlying problem has been resolved. |
This PR is stale because it has been open 365 days with no activity. |
Description
Added a template for a basic PEcAn workflow.
Motivation and Context
Will help users make the transition from Web to interactive R session by adding an Rmarkdown template that users can use to create a new workflow from within the Rstudio interface.
I didn't know where to put the documentation, but it would be
In Rstudio with the PEcAn.all package loaded
library(PEcAn.all)
Types of changes
Checklist: