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

Check in PyTorchModel #79

Merged
merged 25 commits into from
Apr 11, 2023
Merged

Conversation

kathryn-baker
Copy link
Contributor

@kathryn-baker kathryn-baker commented Mar 22, 2023

Implementation of a PyTorchModel class. Currently allows loading model via python objects or from yaml, although loading from yaml requires assigning transformation objects after object creation

@kathryn-baker kathryn-baker marked this pull request as ready for review March 23, 2023 17:08
@kathryn-baker kathryn-baker marked this pull request as draft March 23, 2023 17:46
@kathryn-baker kathryn-baker marked this pull request as ready for review March 28, 2023 19:10
lume_model/pytorch/__init__.py Outdated Show resolved Hide resolved
lume_model/tests/pytorch/__init__.py Outdated Show resolved Hide resolved


def test_model_from_yaml(
rootdir: str, california_model_kwargs: Dict[str, Union[List, Dict, str]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ChristopherMayes are we ok with the file overhead of testing on the California dataset?

Copy link
Collaborator

Choose a reason for hiding this comment

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

its certainly aiding in creating a robust testing suite

lume_model/tests/pytorch/test_model.py Outdated Show resolved Hide resolved
@kathryn-baker kathryn-baker marked this pull request as draft March 30, 2023 19:09
@kathryn-baker kathryn-baker marked this pull request as ready for review March 31, 2023 16:46
@kathryn-baker kathryn-baker marked this pull request as draft April 6, 2023 15:56
@kathryn-baker
Copy link
Contributor Author

A summary of what I've found out relating to the conda-build stage:

  • the reason the build was being cancelled originally is because it couldn't solve the environment so I've removed the tensorflow and botorch requirements for the conda tests
  • I've added an importorskip statement to the keras/torch tests so they won't run in the conda build stage but they should run in the pip stage, which will prevent the conda build from running if they fail so it's not a big loss in the test suite
  • I've done a similar thing for any FileNotFoundErrors, any tests that require files in the conda build stage are skipped but should still be run in the pip stage
  • I've kept the setup.py install call in the build.sh despite the warnings that this method is deprecated because using pip threw an error when I ran it locally and most other example recipes I've seen use the setup.py call

@kathryn-baker kathryn-baker marked this pull request as ready for review April 6, 2023 18:09
@ChristopherMayes ChristopherMayes merged commit fdc9887 into slaclab:master Apr 11, 2023
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.

3 participants