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

Fix sphinx notebook rendering #57

Merged
merged 3 commits into from
Mar 17, 2020
Merged

Conversation

Ohjeah
Copy link
Collaborator

@Ohjeah Ohjeah commented Mar 12, 2020

The current implementation was broken (nothing is currently displayed on sphinx).

I am using a different library now, which has the same look and feel as a regular sphinx gallery.

@Ohjeah Ohjeah requested a review from briandesilva March 12, 2020 15:07
@Ohjeah Ohjeah force-pushed the mq/fix-sphinx-notebooks branch from 02faa83 to bfd9d41 Compare March 12, 2020 15:20
@codecov-io
Copy link

Codecov Report

Merging #57 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
- Coverage   95.52%   95.44%   -0.09%     
==========================================
  Files          18       18              
  Lines         782      768      -14     
==========================================
- Hits          747      733      -14     
  Misses         35       35
Impacted Files Coverage Δ
pysindy/optimizers/base.py 94% <0%> (-0.45%) ⬇️
pysindy/optimizers/sindy_optimizer.py 92.45% <0%> (-0.41%) ⬇️
pysindy/optimizers/stlsq.py 90.9% <0%> (-0.14%) ⬇️
pysindy/feature_library/polynomial_library.py 97.67% <0%> (-0.06%) ⬇️
pysindy/feature_library/custom_library.py 98% <0%> (-0.04%) ⬇️
pysindy/pysindy.py 98.17% <0%> (-0.02%) ⬇️
pysindy/feature_library/feature_library.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7ff690...bfd9d41. Read the comment docs.

@briandesilva
Copy link
Member

Any idea why the complexity test is failing for just Python 3.8? It looks like SR3 with l1 regularization produces a slightly more complex model than Lasso. I'm not sure why this would only be the case in Python 3.8 -- maybe it just has to do with the value of random_state that's generated for that particular test.

In any case, maybe instead of comparing complexity across optimizers, we should look at complexity for a fixed model as the regularization parameter is changed? We could loop over each optimizer and check that as the regularization parameter is increased, the model becomes less complex.

Another fix would be to increase the default value of threshold in SR3.

@Ohjeah
Copy link
Collaborator Author

Ohjeah commented Mar 13, 2020

We could also relax the strict complexity ordering and allow the less complex model to be within +- 1 of the more complex model.

@Ohjeah
Copy link
Collaborator Author

Ohjeah commented Mar 17, 2020

@briandesilva I added the test you suggested and also relaxed the strict condition on my test to account for noise and non-normalized alpha parameters

@briandesilva briandesilva merged commit 6fa36dd into master Mar 17, 2020
@briandesilva briandesilva deleted the mq/fix-sphinx-notebooks branch March 17, 2020 17:24
jpcurbelo pushed a commit to jpcurbelo/pysindy_fork that referenced this pull request Apr 30, 2024
jpcurbelo pushed a commit to jpcurbelo/pysindy_fork that referenced this pull request May 9, 2024
This commit unifies the way the different models preprocess the static and dynamic input data.

Closes dynamicslab#57 and dynamicslab#74, part of dynamicslab#64.

Similar to the model head, there is now an InputLayer that all (single-frequency) models use. Depending on the config, this layer will:
- pass the static variables through an embedding layer (if cfg.statics_embedding is True)
- pass the dynamic variables through an embedding layer (if cfg.dynamics_embedding is True)
- concatenate static to each step of dynamic inputs (unless this is deactivated; e.g., for EA-LSTM this is not wanted)

Key changes:
- New config arguments statics_embedding, dynamics_embedding (both bool, default False).
- EmbCudaLSTM is no longer needed since the functionality is now part of CudaLSTM. For backwards compatibility, the class still exists but defers to CudaLSTM.
- If EA-LSTM and static embeddings are used, there will now be a linear layer between the embedding output and the lstm input to make sure the dimensions match. Previously, the linear layer would be left out if static embeddings were used.
jpcurbelo pushed a commit to jpcurbelo/pysindy_fork that referenced this pull request May 9, 2024
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