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

Jenn #517

Merged
merged 15 commits into from
Feb 26, 2024
Merged

Jenn #517

merged 15 commits into from
Feb 26, 2024

Conversation

shb84
Copy link
Contributor

@shb84 shb84 commented Feb 21, 2024

Summary

This PR replaces the neural_net package by jenn as an upstream dependency, which is an updated version that is about 5x faster. The previous implementation genn inadvertently copied data which has been fixed in jenn by updating arrays in place.

Details

Major changes:

  • Added jenn>=1.0.1 as upstream dependency in requirements.txt
  • Removed stm.surrogate_models.neural_net
  • Using jenn.model.NeuralNet instead of smt.utils.neural_net.model.Model

Minor changes:

  • Removed:
    • genn.load_smt_data function replaced by genn.GENN.load_data method
    • stm.examples.multi_modal b/c it used API of underlying GENN model rather than SMT API (confusing to users)
    • plot_training_history and goodness_of_fit because they did not conform to SMT API
    • genn.run_example because it duplicates the unit test / usage example in docs
    • smt.tests.test_all.genn function b/c it wasn't really needed and stood out relative to the other surrogates
    • @unittest.skipIf(int(os.getenv("RUN_SLOW", 0)) < 1, "too slow") b/c jenn should fix issue with slowness
    • _smt.utils.neural_net since it is replaced by jenn
    • GENN tolerance relaxations in test_all.py, except for exp which could only be reduced to 10%
  • Changed:
    • Replaced deep, wide by hidden_layer_sizes (GENN class options)
    • Added is_normalize (GENN class options)
    • Added is_bracktracking (GENN class options)
    • genn docs sections (adjusted for GENN class modifications)
    • learning_airfoil_parameters example (adjusted for GENN class modifications)
  • Added:
    • jenn as required package in setup.py
    • (suggestion) environment.yml for development convenience conda env update -f environment.yml -n smt

@shb84 shb84 closed this Feb 22, 2024
@shb84 shb84 reopened this Feb 22, 2024
@shb84
Copy link
Contributor Author

shb84 commented Feb 22, 2024

Running into issues on ci which is complaining that a package I removed is not present. Please help.

'language_level' not set, using '3str' for now (Py3). This has changed from earlier releases! File: /home/runner/work/smt/smt/smt/src/rmts/rmtsclib.pyx
        tree = Parsing.p_module(s, pxd, full_module_name)
      Compiling smt/src/rmts/rmtsclib.pyx because it changed.
      [1/1] Cythonizing smt/src/rmts/rmtsclib.pyx
      running egg_info
      creating smt.egg-info
      writing smt.egg-info/PKG-INFO
      writing dependency_links to smt.egg-info/dependency_links.txt
      writing requirements to smt.egg-info/requires.txt
      writing top-level names to smt.egg-info/top_level.txt
      writing manifest file 'smt.egg-info/SOURCES.txt'
      error: package directory 'smt/utils/neural_net' does not exist
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build editable did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.
Error: Process completed with exit code 1.

@relf
Copy link
Member

relf commented Feb 22, 2024

Hi. I pulled your PR locally and run pytest (after installing jenn), I got an error on the import, see below

>python
Python 3.8.5 (default, Sep  3 2020, 21:29:08) [MSC v.1916 64 bit (AMD64)] :: Anaconda, Inc. on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from jenn.model import NeuralNet
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'jenn.model'
>>>       

Then I noticed that pip install -U jenn installs jenn 0.1.0 while I see there is a jenn 1.0.0.
So I tried:

>pip install jenn==1.0.0
ERROR: Ignored the following versions that require a different python version: 1.0.0 Requires-Python >=3.11
ERROR: Could not find a version that satisfies the requirement jenn==1.0.0 (from versions: 0.0.8, 0.1.0)
ERROR: No matching distribution found for jenn==1.0.0

As for now SMT is tested on py3.9, py3.10, py3.11, so...

@shb84
Copy link
Contributor Author

shb84 commented Feb 22, 2024

Thank you, Rémy. That makes sense. I did require Python 11 or greater on jenn, as I hadn't yet tested it on lower versions, which would explain. I will relax that requirement, do some testing, and circle back. This is very helpful. Thanks again.

@shb84
Copy link
Contributor Author

shb84 commented Feb 25, 2024

Ready for review.

Copy link
Member

@relf relf left a comment

Choose a reason for hiding this comment

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

Hi Steven. This looks good to me, thank you!

@relf relf merged commit ff1e268 into SMTorg:master Feb 26, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants