ENH: Raise explicit error when FROLS has linearly dependent columns. #193
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jpcurbelo
pushed a commit
to jpcurbelo/pysindy_fork
that referenced
this pull request
Apr 30, 2024
…dependence-err ENH: Raise explicit error when FROLS has linearly dependent columns.
jpcurbelo
pushed a commit
to jpcurbelo/pysindy_fork
that referenced
this pull request
May 9, 2024
…ure an up-to-date xarray. (dynamicslab#193)
jpcurbelo
pushed a commit
to jpcurbelo/pysindy_fork
that referenced
this pull request
May 9, 2024
* Fix typo in models doc (dynamicslab#190) * CPU environment now pulls from conda forge, which is necessary to ensure an up-to-date xarray. (dynamicslab#193) * Fix handling of weekly frequencies. (dynamicslab#194) From neuralhydrology/neuralhydrology#111: pd.infer_freq will return strings like "W-SUN" for weekly data, which pd.Timestamp doesn't understand. We now convert these frequencies to their equivalent multiple of 7D. * Correcting the pre model hook and UMAL sampling (dynamicslab#195) * The current use of `pre_model_hook` only applied the hook in training. This can be useful, but UMAL requires the hook also in validation and test. Thus, with the old setup UMAL only worked for the training. My propsoed changes make the `pre_model_hook` part of the model, apply it everywhere, and allow UMAL validation and evaluation. I think in the future we should also allow for different hook behaviors according to the setting at which it is called. But, for now the proposed changes are enough. * Simpler Hook and Cleaner Pipeline This commit comprises two things. (1) A pre model hook that is simpler than the one in original PR. (2) An idea to avoid copying the whole dataset that just involves a copy of the labels. That is still suboptimal in terms of memory use, but make the overall code simpler. Not sure if it is the best version. However, at some point we have to extend the labels for the loss, and we need to do so withouth breaking the whole downstream procedure. So maybe this is a good middle ground. * Simplified UMAL Sampling Logic This commit implements an idea from Martin that simplifies the sampling logic of the UMAL sampling util so that the sampling automatically checks whether the data has been extended. Thus no extra argument is required for the function call for `sample_umal`. * Spell correction neuralhydrology/modelzoo/basemodel.py Updated comment so that additional is spelled correctly Co-authored-by: Martin Gauch <15731649+gauchm@users.noreply.github.com> * Spell correction for neuralhydrology/utils/samplingutils.py UMAl -> UMAL Co-authored-by: Martin Gauch <15731649+gauchm@users.noreply.github.com> * Imporvement for Comments Added some minor changes to the comments. --------- Co-authored-by: Martin Gauch <15731649+gauchm@users.noreply.github.com> * Update __about__.py --------- Co-authored-by: Martin Gauch <15731649+gauchm@users.noreply.github.com> Co-authored-by: Grey Nearing <grey.nearing@gmail.com> Co-authored-by: Daniel Klotz <klotz@ml.jku.at>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test added: test_frols_error_linear_dependence().
Test fails on master, passes on this branch. Fixes #191