Skip to content

Conversation

lamorton
Copy link
Contributor

@lamorton lamorton commented Jul 2, 2021

This addresses #1082. AbstractSystems are now categorized into 3 classes (0, 1, N independent variables). The independent_variable method has been replaced with independent_variables which returns any/all independent variables for any type of system. Use get_iv where it is known that only one independent variable exists (ie, for AbstractTimeDependentSystems).

@lamorton
Copy link
Contributor Author

lamorton commented Jul 2, 2021

NeuralPDE integration test is failing b/c I changed PDESystem.indvars to PDESystem.ivs. I've got a PR read to deal with that. Is there a way to deprecate access like this?

Also, the @deprecate independent_variables seems not to be providing a warning, and I don't understand why.

lamorton and others added 3 commits July 3, 2021 13:19
Co-authored-by: Christopher Rackauckas <accounts@chrisrackauckas.com>
Co-authored-by: Christopher Rackauckas <accounts@chrisrackauckas.com>
lamorton added 3 commits July 17, 2021 12:05
…endence

# Conflicts:
#	docs/src/systems/SDESystem.md
#	src/ModelingToolkit.jl
#	src/structural_transformation/StructuralTransformations.jl
@lamorton
Copy link
Contributor Author

I think the NeuralPDE test failure is unrelated.

lamorton added 2 commits July 25, 2021 13:40
…lly weight-bearing downstream in DataDrivenDiffEq.

This reverts commit 98da595.
@lamorton
Copy link
Contributor Author

Looks like that fallback method for independent_variables was important after all: DataDrivenDiffEq declares Basis<:AbstractSystem. I'm adding that method back & putting some tests for it to meet the coverage tests.

@lamorton
Copy link
Contributor Author

lamorton commented Aug 1, 2021

Is the coverage the only thing preventing this from being merged?

@ChrisRackauckas
Copy link
Member

No, I think it was just JuliaCon 😅 . I'd like @YingboMa to take a look first before merging.

```
"""
struct PDESystem <: ModelingToolkit.AbstractSystem
struct PDESystem <: ModelingToolkit.AbstractMultivariateSystem
Copy link
Member

Choose a reason for hiding this comment

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

Remove ModelingToolkit.

@YingboMa
Copy link
Member

YingboMa commented Aug 4, 2021

I think this is good to merge, though, please squash the commits appropriately.

Also, we should merge other bug fixes PR and do a patch/minor release before merging this PR. After this PR, and together with #1157, we should do a new major release.

PS: If you are using git from the command line, you can always do git commit --amend if you want to fix a small typo with making a new commit.

@YingboMa YingboMa merged commit 89fdee8 into SciML:master Aug 6, 2021
@lamorton
Copy link
Contributor Author

lamorton commented Aug 6, 2021

@YingboMa Thanks for taking care of the loose ends here.

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