10000 Parameterize model validation and comparison by MoLi7 · Pull Request #168 · USEPA/useeior · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Parameterize model validation and comparison #168

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 4, 2021
Merged

Parameterize model validation and comparison #168

merged 3 commits into from
Nov 4, 2021

Conversation

MoLi7
Copy link
Collaborator
@MoLi7 MoLi7 commented Oct 22, 2021

The idea is to create a standard process to validate and compare models, then parameterize the process based on modelname to render model-specific validation and comparison reports.

The reports will be forced to md format for version control purpose and can be later converted to PDF or other formats to meet different requirements.

This PR is intended to resolve issue #165.

@bl-young
Copy link
Collaborator

@MoLi7 can you clarify how I go about testing these scripts? Are they supposed to be built in to model development?

@MoLi7
Copy link
Collaborator Author
MoLi7 commented Oct 25, 2021

@bl-young you should always run/knit _render.Rmd to validate/compare models - it will call ValidateModel.Rmd and CompareModel.Rmd to execute the actions. Before you knit _render.Rmd, make sure the models you want to look at are declared in the parameter field in the YAML header, while the models you are not interested in are commented out.

I consider them internal QAs (and/or preliminary analyses) but not essential model components that will be exposed to users.

@bl-young
Copy link
Collaborator

@MoLi7 I get this error (and a similar one on Compare...render.rmd)

Error in abs_path(input) : 
  The file 'inst/doc/ValidateModel.Rmd' does not exist.
Calls: <Anonymous> ... eval -> <Anonymous> -> setwd -> dirname -> abs_path
Execution halted```

@MoLi7
Copy link
Collaborator Author
MoLi7 commented Oct 25, 2021

Do you not have ValidateModel.Rmd in inst/doc/?

@bl-young
Copy link
Collaborator

I do...which is strange. I'm on the param_val branch and the file is there. Something about the path is not working for me locally.

@bl-young
Copy link
Collaborator

@MoLi7 ok the issue was that by default it was knitting based on the document directoyr not the project directory. now it works. sorry!

@bl-young
Copy link
Collaborator

Looks good to me, except I can't complete comparison of 2.0 and 2.1

Error in Ops.data.frame(A, B) : 
  '-' only defined for equally-sized data frames
Calls: <Anonymous> ... eval -> eval -> compareFlowTotals -> Ops.data.frame
Execution halted```

@MoLi7
Copy link
Collaborator Author
MoLi7 commented Oct 25, 2021

I guess you haven't downloaded Water_national_2015_m3_v0.3.2_b67a100.parquet from flowsa folder 😉
See Catherine's reply here.

@bl-young
Copy link
Collaborator

No I have that file and can build 2.1 just fine

@MoLi7
Copy link
Collaborator Author
MoLi7 commented Oct 25, 2021

Sorry that I assumed you did not have it. It should be due to flow differences in 2.0 and 2.1 - that's why the error message is about equally-sized data frames. I will modify compareFlowTotals to adapt to such comparison.

@MoLi7
Copy link
Collaborator Author
MoLi7 commented Oct 25, 2021

1dd1154 enables comparison between models that have different flows and prints out what the flows are.

WesIngwersen
WesIngwersen approved these changes Nov 3, 2021
View reviewed changes
Copy link
Collaborator
@WesIngwersen WesIngwersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize this one was waiting on me. It's good sorry,

@MoLi7 MoLi7 merged commit a7f170e into develop Nov 4, 2021
@MoLi7 MoLi7 deleted the param_val branch November 4, 2021 00:37
@MoLi7 MoLi7 mentioned this pull request Dec 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WesIngwersen WesIngwersen WesIngwersen approved these changes

@bl-young bl-young bl-young approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MoLi7 @bl-young @WesIngwersen
0