Skip to content
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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

dlebauer
Copy link
Member

@dlebauer dlebauer commented Oct 20, 2017

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.

image

I didn't know where to put the documentation, but it would be

In Rstudio with the PEcAn.all package loaded

library(PEcAn.all)
  • File --> new file --> Rmarkdown --> From Template
  • Choose the PEcAn workflow template.
  • You can start with a pecan.xml file from the web interface or the tests/ directory and modify as needed

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.
  • 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.

Copy link
Contributor

@tonygardella tonygardella left a 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")
```

Copy link
Contributor

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')

Copy link
Member

@mdietze mdietze left a 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.

@dlebauer
Copy link
Member Author

@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 ...

image

Merge remote-tracking branch 'origin' into rmd_template

# Conflicts:
#	CHANGELOG.md
@tonygardella
Copy link
Contributor

@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.

@infotroph
Copy link
Member

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 PEcAn.workflow instead of PEcAn.all?

@tonygardella
Copy link
Contributor

@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.

@dlebauer dlebauer deleted the rmd_template branch August 31, 2020 18:36
@dlebauer dlebauer restored the rmd_template branch December 3, 2020 17:41
@dlebauer
Copy link
Member Author

dlebauer commented Dec 3, 2020

I am re-opening because I don't think that the underlying problem has been resolved.

@dlebauer dlebauer reopened this Dec 3, 2020
@github-actions
Copy link

github-actions bot commented Dec 4, 2021

This PR is stale because it has been open 365 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants