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

152 make tensor flow optional #154

Merged
merged 5 commits into from
Aug 7, 2024
Merged

152 make tensor flow optional #154

merged 5 commits into from
Aug 7, 2024

Conversation

teubert
Copy link
Contributor

@teubert teubert commented Jul 30, 2024

This PR accomplishes 2 things

  1. Makes installing tensor flow optional using [datadriven] option, and
  2. Updates configuration from old setup.py to new pyproject.toml

Now to install with tensorflow you must use the following command
pip install progpy[datadriven] or pip install '.[datadriven]' (for local copy)

Also updated LSTM to give a more descriptive error if tensor flow isn't installed and updated documentation of LSTM to add note.

How to test:

  • Create a new venv
  • Install progpy local copy (pip install -e .) , run tests. data tests should fail and give useful error.
  • Clear out venv
  • install progpy local copy, with datadriven (pip install -e '.[datadriven]'), run tests. all should pass

@teubert teubert self-assigned this Jul 30, 2024
Copy link

Thank you for opening this PR. Each PR into dev requires a code review. For the code review, look at the following:

  • Reviewer (someone other than author) should look for bugs, efficiency, readability, testing, and coverage in examples (if relevant).
  • Ensure that each PR adding a new feature should include a test verifying that feature.
  • All errors from static analysis must be resolved.
  • Review the test coverage reports (if there is a change) - will be added as comment on PR if there is a change
  • Review the software benchmarking results (if there is a change) - will be added as comment on PR
  • Any added dependencies are included in requirements.txt, setup.py, and dev_guide.rst (this document)
  • All warnings from static analysis must be reviewed and resolved - if deemed appropriate.

@teubert teubert added components: data model project: SWS System Wide Safety Project labels Jul 30, 2024
This was linked to issues Jul 30, 2024
@teubert
Copy link
Contributor Author

teubert commented Jul 30, 2024

Unfortunately, the way things are setup the new test config will not take effect until this is merged. So surrogates and data tests will fail and have to be tested manually

@jason-watkins
Copy link

Seems to mostly be working, but I do get this error on (I think) all of the tensorflow tests:

======================================================================
ERROR: test_lstm_simple (tests.test_data_model.TestDataModel)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jason/code/nasa/progpy/tests/test_data_model.py", line 127, in test_lstm_simple
    m = self._test_simple_case(LSTMStateTransitionModel, window=5, epochs=20, max_error=3)
  File "/home/jason/code/nasa/progpy/tests/test_data_model.py", line 48, in _test_simple_case
    m2 = DataModelType.from_data(
  File "/home/jason/code/nasa/progpy/src/progpy/data_models/lstm_model.py", line 598, in from_data
    history = model.fit(
  File "/home/jason/code/nasa/progpy_venv/lib/python3.10/site-packages/keras/src/utils/traceback_utils.py", line 122, in error_handler
    raise e.with_traceback(filtered_tb) from None
  File "/home/jason/code/nasa/progpy_venv/lib/python3.10/site-packages/keras/src/utils/traceback_utils.py", line 117, in error_handler
    return fn(*args, **kwargs)
TypeError: TensorFlowTrainer.fit() got an unexpected keyword argument 'workers'

Could be related to the fact that I am running this in Windows WSL?

@teubert
Copy link
Contributor Author

teubert commented Jul 30, 2024

Seems to mostly be working, but I do get this error on (I think) all of the tensorflow tests:


======================================================================

ERROR: test_lstm_simple (tests.test_data_model.TestDataModel)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/jason/code/nasa/progpy/tests/test_data_model.py", line 127, in test_lstm_simple

    m = self._test_simple_case(LSTMStateTransitionModel, window=5, epochs=20, max_error=3)

  File "/home/jason/code/nasa/progpy/tests/test_data_model.py", line 48, in _test_simple_case

    m2 = DataModelType.from_data(

  File "/home/jason/code/nasa/progpy/src/progpy/data_models/lstm_model.py", line 598, in from_data

    history = model.fit(

  File "/home/jason/code/nasa/progpy_venv/lib/python3.10/site-packages/keras/src/utils/traceback_utils.py", line 122, in error_handler

    raise e.with_traceback(filtered_tb) from None

  File "/home/jason/code/nasa/progpy_venv/lib/python3.10/site-packages/keras/src/utils/traceback_utils.py", line 117, in error_handler

    return fn(*args, **kwargs)

TypeError: TensorFlowTrainer.fit() got an unexpected keyword argument 'workers'

Could be related to the fact that I am running this in Windows WSL?

Well that's frustrating. It should be working on windows. But tensorflow is VERY sensitive to os and version. I'll open a new issue to investigate that.

I'll also be updating to use keras 3 for this release. That might fix this issue too. But I'll open a bug report to track it

@teubert teubert merged commit f030eb8 into dev Aug 7, 2024
27 of 30 checks passed
@teubert teubert deleted the 152-make-tensor-flow-optional branch August 7, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
components: data model project: SWS System Wide Safety Project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to pyproject.toml Make tensor flow optional
2 participants