search

USEPA / AMET

Code base for the U.S. EPA’s Atmospheric Model Evaluation Tool (AMET)
https://www.epa.gov/CMAQ/atmospheric-model-evaluation-tool
21 stars 21 forks source link

Develop #120

Closed lizadams closed 2 years ago

lizadams commented 2 years ago

I have made updates to the documentation. Please review and let me know if they are ok. @wkappel I am not sure how to tag Rob Gilliam for his reveiw.

wkappel commented 2 years ago

Hi Liz,

Is there an easy way to view all the changes together rather than each individual pull request? I wasn't able to go straight to the full documents in your repo, which I guess makes sense. I can only view the changes.

It might be easier to just merge these all into develop and make any needed changes there. What do you think?

Wyat

lizadams commented 2 years ago

This is one method, to review the changes by file. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

wkappel commented 2 years ago

Thanks, Liz.

These updates look good. Looking through them I realized that I don't think I use the EXEC_sitex_daily_config or EXEC_sitex_config variables specified in the amet-config.R file anymore. The path to those executables is set in the run script now. So, I think they can safely be removed from the config file.

For now, I would say merge these changes to the documentation (not sure if Rob will review them or not; his Github name is coastwx by the way). Then, I will update the amet-config.R file to remove those variables, and also update the documentation to reflect their removal. Does that sound good?

Wyat

wkappel commented 2 years ago

Well, now that I take a closer look I actually do use the EXEC_sitex_config and EXEC_sitex_daily_config as "backup" variables in case those paths are not specified in the run script.

I don't love having those two AQ analysis run scripts. Basically, the pre and post script can be configured to do the same things as the post only script. It seems I should just drop the post only. I'll look into doing that.

Wyat

wkappel commented 2 years ago

Seems I may have caused a conflict by deleting the aqProject_post_only.csh script from the develop repo.

Is the simplest way to fix that to also just delete it from your repo, Liz?

Wyat