Closed ajleeson closed 11 months ago
Aurora,
Let's rethink this before I merge your pull request. I'd like to get rid of the dependence on Ldir['traps_name'], probably just hard-coding the name in the various places, although it would be better to just change it in one place. Perhaps the LO/pre/trapsV00 code could look for its own directory name (trapsV00 in this case) and use this for output naming.
I would also like to separate this from the LO/forcing code that uses is. Then for the forcing code you would want to have one place (only), like make_forcing_main.py, where you specify which LO_output/pre/ traps version to use.
Hi Parker,
I agree. Let's come up with a better naming scheme. Perhaps trapsP## for the traps pre-processing code, and trapsF## for the traps forcing version. When I get back from my travels, I'll find a better place to weave these names into the traps code. By that time I'll also find a place for the main traps readme and Ecology data processing script. I'm glad to know that my pull request worked, so I'm now at least familiar with the process. I'll send you another pull request in mid-December with the updates mentioned above. Let me know if you have any other thoughts before then!
Thanks, Aurora
P.S. I responded to your message in my email, but I think the reply gets sent to your GitHub(?). Hopefully it works. Let me know if you receive this message.
On Fri, Dec 1, 2023 at 8:41 AM Parker MacCready @.***> wrote:
Aurora,
Let's rethink this before I merge your pull request. I'd like to get rid of the dependence on Ldir['traps_name'], probably just hard-coding the name in the various places, although it would be better to just change it in one place. Perhaps the LO/pre/trapsV00 code could look for its own directory name (trapsV00 in this case) and use this for output naming.
I would also like to separate this from the LO/forcing code that uses is. Then for the forcing code you would want to have one place (only), like make_forcing_main.py, where you specify which LO_output/pre/ traps version to use.
— Reply to this email directly, view it on GitHub https://github.com/parkermac/LO/pull/5#issuecomment-1836438260, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADYYQ22LI3RNLV6XIDHZDZTYHICDRAVCNFSM6AAAAAA76VGBY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZWGQZTQMRWGA . You are receiving this because you authored the thread.Message ID: @.***>
got your message - sounds good. Have fun on your travels!
On Fri, Dec 1, 2023 at 3:57 PM ajleeson @.***> wrote:
Hi Parker,
I agree. Let's come up with a better naming scheme. Perhaps trapsP## for the traps pre-processing code, and trapsF## for the traps forcing version. When I get back from my travels, I'll find a better place to weave these names into the traps code. By that time I'll also find a place for the main traps readme and Ecology data processing script. I'm glad to know that my pull request worked, so I'm now at least familiar with the process. I'll send you another pull request in mid-December with the updates mentioned above. Let me know if you have any other thoughts before then!
Thanks, Aurora
P.S. I responded to your message in my email, but I think the reply gets sent to your GitHub(?). Hopefully it works. Let me know if you receive this message.
On Fri, Dec 1, 2023 at 8:41 AM Parker MacCready @.***> wrote:
Aurora,
Let's rethink this before I merge your pull request. I'd like to get rid of the dependence on Ldir['traps_name'], probably just hard-coding the name in the various places, although it would be better to just change it in one place. Perhaps the LO/pre/trapsV00 code could look for its own directory name (trapsV00 in this case) and use this for output naming.
I would also like to separate this from the LO/forcing code that uses is. Then for the forcing code you would want to have one place (only), like make_forcing_main.py, where you specify which LO_output/pre/ traps version to use.
— Reply to this email directly, view it on GitHub https://github.com/parkermac/LO/pull/5#issuecomment-1836438260, or unsubscribe < https://github.com/notifications/unsubscribe-auth/ADYYQ22LI3RNLV6XIDHZDZTYHICDRAVCNFSM6AAAAAA76VGBY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZWGQZTQMRWGA>
. You are receiving this because you authored the thread.Message ID: @.***>
— Reply to this email directly, view it on GitHub https://github.com/parkermac/LO/pull/5#issuecomment-1836936774, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC5WRX6FYYYKIIX5RZXPIRTYHJVE7AVCNFSM6AAAAAA76VGBY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZWHEZTMNZXGQ . You are receiving this because you commented.Message ID: @.***>
--
Parker MacCready
Leo Maddox Endowed Professor in Oceanography
Research Professor, School of Oceanography I live and work on the land and waters of the Squaxin Island Tribe. Email: @.** URL: faculty.washington.edu/pmacc LiveOcean Daily Forecasts http://faculty.washington.edu/pmacc/LO/LiveOcean.html Cell: (360) 359-1936, Office: (206) 685-9588 pronouns: he/him*
Aurora,
I would like to put off approving this pull request until you remove the edit to get_lo_info.py (where "traps_name" is set). The reason is that this is currently part of my workflow for the long hindcast I am doing using traps00. I think we need a more contained solution to this. Maybe you can make the pre/trapsV00/makeclimatology[].py code look to see which traps folder they are in and use this? And then do something different in the forcing code.
We can sit down and talk about this in January. No hurry.
PM
Hi Parker,
Thanks for your note on this. I was also thinking about having the code check the traps folder name to identify which version of traps to use, and thus what to name the output folder for climatology files, etc. It's reassuring that we had similar thoughts! I'm working on implementing these changes in the forked LO repo, and hope to send you a pull request before January. I'd be happy to discuss changes then as well.
Happy Holidays! Aurora
On Tue, Dec 19, 2023 at 8:12 AM Parker MacCready @.***> wrote:
Aurora,
I would like to put off approving this pull request until you remove the edit to get_lo_info.py (where "traps_name" is set). The reason is that this is currently part of my workflow for the long hindcast I am doing using traps00. I think we need a more contained solution to this. Maybe you can make the pre/trapsV00/makeclimatology[].py code look to see which traps folder they are in and use this? And then do something different in the forcing code.
We can sit down and talk about this in January. No hurry.
PM
— Reply to this email directly, view it on GitHub https://github.com/parkermac/LO/pull/5#issuecomment-1863064620, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADYYQ275PV2B7MHFU3SSPCTYKG4IRAVCNFSM6AAAAAA76VGBY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRTGA3DINRSGA . You are receiving this because you authored the thread.Message ID: @.***>
Hi Parker,
I just finished a re-structure of the TRAPS code base to allow for more modularity (and hopefully make it easier and more straightforward to have different versions of TRAPS on your machine). In this pull request, I also propose to add all of the TRAPS READMEs to the LO repo. Once these changes are approved and merged into LO, I will add a note to my LO_traps repo stating that the repo is obsolete with a link that points to the LO version of TRAPS.
Note that I have not made any changes to the TRAPS scripts, I have only restructured the code. I have also completed a test run of all the scripts to ensure that the new re-structure still works.
The details of changes made in this pull request are listed below:
As part of this change, I have removed the Ldir[‘traps_name’] convention throughout the code (including from get_lo_info.py). I have replaced this old naming convention with a new naming convention which also makes the TRAPS code more modular.
trapsD00 (D for data) trapsD00 is the new folder name for Ecology’s raw data stored in Aurora’s perigee account in LO_data. If Ecology ever releases an updated dataset, then I will create a new LO_data folder called trapsD01, and increment upwards from there. All users will still need to copy the files from LO_data/trapsD00 from perigee onto their local and remote machines.
trapsP00 (P for pre) trapsP00 is the new folder name for TRAPS scripts stored in LO/pre. Now, each of the climatology scripts (e.g. make_climatology_tinrivs.py) will check to see what folder name it is saved in. For this version, the folder name is trapsP00. Then, the scripts will save climatology files in LO_output/pre/trapsP00. For new versions of traps climatology generation, I will also increment the folder name by one to be LO/pre/trapsP01, and outputs will be automatically saved to LO_output/pre/trapsP01. I have also added a config file called traps_data_ver.csv. In it, I list the version of Ecology data to use (in this case, trapsD00). The climatology scripts and traps_placement script all reference this config file to get the name of the LO_data folder to use. Thus, if a new version of Ecology data is released, users will only need to change the name of the data folder in the traps_data_ver.csv config file (to be trapsD01, or whatever the new version is named).
I have also added a script called ecology_excel2netCDF.py. Previously, I did not include this in the LO_traps repo because users do not need to use it. However, I have added it now for completion. This script converts all of Ecology’s raw data from on excel file per source to two .nc files (these are the all_nonpoint_source_data.nc and all_point_source_data.nc files stored in LO_data). This script also references the config file to get the correct version of trapsD##. In theory, users still do not need to run this script (they can simply copy the .nc files from my LO_data/trapsD## folder on perigee).
trapsF00 (F for forcing) I have also changed the name of the traps forcing folder to follow the new naming convention. The most up-to-date version of TRAPS forcing is now saved in LO/forcing/trapsF00, and I will increment any new versions (i.e. trapsF01). One issue that came up during development is that trapsF00 needs to know which version of trapsP## to use. I solved this problem by adding an argument in driver_forcing3.py called -tP (long name is –trapsP). Now, when users generate forcing, they can specify the version of trapsP## to use. By default, -tP uses an input of trapsP00. Based on the version of trapsP##, the forcing script can also access the trapsP## folder’s traps_data_ver.csv config file to get the appropriate version of trapsD## to use (which is necessary for details such as WWTP open/close dates).
We lost some of the TRAPS READMEs when we integrated TRAPS into LO. To resolve this issue, I have created a new folder called LO/traps_notes. My plan is to store ALL traps readmes in this folder, and only store them in this location. README’s in LO/pre/trapsP00 and LO/forcing/trapsF00 now solely have a hyperlink that points to the readmes in LO/traps_notes. My reasoning behind this change is that all READMEs for TRAPS will be easy to locate since they will be stored in one location. Also, we will not need to worry about generating duplicate readmes every time I create a new trapsF## or trapsP## folder. This makes version control of readmes easier to maintain.
Note that I have also updated the READMEs to reflect the restructuring changes I discussed above.
Let me know if you have any questions or concerns. Happy New Year’s! Aurora
Updated get_lo_info and folder names to change traps_name from traps00 to trapsV00.
The reason for this is because I already have an older version of traps called traps00 on my computer that was used during the debugging process. The name trapsV00 will get rid of any confusion. We can think of the "V00" as the first functioning version of traps.