NOAA-OWP / lstm

Other
12 stars 17 forks source link

lstm_package #34

Closed peckhams closed 1 year ago

peckhams commented 2 years ago

Created a Python package for the LSTM model (within lstm folder) and tested it within NextGen. Also bug fixes, extra docs and improved Jupyter notebooks.

Additions

Removals

Changes

Testing

  1. Jupyter notebooks ran successfully.
  2. Was able to run this new LSTM repo for all HUC01 catchments on a Mac within NextGen.

Screenshots

Notes

Todos

-

Checklist

Testing checklist

Target Environment support

Accessibility

Other

madMatchstick commented 2 years ago

Also bug fixes, extra docs and improved Jupyter notebooks.

@peckhams Which bugs? Fix #32 & Fix #29 ?

madMatchstick commented 2 years ago

Created a Python package for the LSTM model (within lstm folder) and tested it within NextGen.

@peckhams Which, if any "standalone", scripts did you test?

madMatchstick commented 2 years ago

We will need to edit README.md, specifically Running BMI LSTM, info, links, etc. @peckhams Can you expand more on standalone mode here?

Also, README2.md is more focused on lstm_bmi as a package, perhaps rename?

peckhams commented 2 years ago

Also bug fixes, extra docs and improved Jupyter notebooks.

@peckhams Which bugs? Fix #32 & Fix #29 ?

Yes, #32 is fixed, and #29 was fixed previously and should still be.

peckhams commented 2 years ago

Created a Python package for the LSTM model (within lstm folder) and tested it within NextGen.

@peckhams Which, if any "standalone", scripts did you test?

Do you mean a script that tests the LSTM model in "standalone mode"? If so, when you run LSTM via: % python -m lstm, it runs entirely outside of NextGen and uses "__main__.py" in the lstm package folder. Most of the testing was done running LSTM inside of NextGen, however.

peckhams commented 2 years ago

We will need to edit README.md, specifically Running BMI LSTM, info, links, etc. @peckhams Can you expand more on standalone mode here?

Also, README2.md is more focused on lstm_bmi as a package, perhaps rename?

Yes, we should update README.md, and should probably rename README2.md to something more descriptive. Let's discuss this in our next LSTM call.

madMatchstick commented 2 years ago

Created a Python package for the LSTM model (within lstm folder) and tested it within NextGen.

@peckhams Which, if any "standalone", scripts did you test?

Do you mean a script that tests the LSTM model in "standalone mode"? If so, when you run LSTM via: % python -m lstm, it runs entirely outside of NextGen and uses "main.py" in the lstm package folder. Most of the testing was done running LSTM inside of NextGen, however.

Thanks for clarifying, I am running into some pathing mishaps with this "standalone" approach. I'll continue to work through and post any major roadblocks here. -thx

madMatchstick commented 1 year ago

@peckhams why was input variable atmosphere_water__time_integral_of_precipitation_mass_flux changed back to atmosphere_water__liquid_equivalent_precipitation_rate?