- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 233
Support Symbolic Arrays of arbitrary length #3107
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
Support Symbolic Arrays of arbitrary length #3107
Conversation
cdc3d69    to
    5239e4f      
    Compare
  
    5239e4f    to
    bcf38f2      
    Compare
  
    | The tests pertaining to Model-parsing (model_parsing and in dq_units and units)have passed. The failure looks unrelated. | 
Update metadata dict and kwargs for symbolic arrays
- Ensure `@variables` are variables of an independent var. - In both `@variables` and `@parameters` case, ensure a single indpendent var is used
With many ways to define, this feature demands a dedicated section.
While here, removes passing `indices` around to generate_var, update_kwargs_and_metadata! and parse_variable_def! as these no longer handles symbolic-arrays.
bcf38f2    to
    1290985      
    Compare
  
    | This PR is rebased off the master and NO_VALUE changes are included. | 
| @test MockModel.structure[:defaults] == Dict(:n => 1.0, :n2 => "g()") | ||
| end | ||
|  | ||
| @testset "Arrays using vanilla-@variable syntax" begin | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing the test using structural parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @parameters begin | ||
| p1[1:4] | ||
| p2[1:N] | ||
| p3[1:N, 1:M] = 10, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this one substantially more difficult to support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really.
Parser is now more granular. It picks up relevant bits of expression at the parsing step.
I've tried to batch similar ones together (example) and code is in a that is simpler to maintain.
Actually, let me add doc-strings to each parser step to indicate the syntax it parses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added the doc strings matching the block here: 4c0b1ed
This is handled in two steps and this block is never used
37bab5c    to
    451bafd      
    Compare
  
    451bafd    to
    4c0b1ed      
    Compare
  
    | The doc failure is unrelated (errors stem from initialization.md) | 
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Following is now supported:
A dedicated section, in the docs, describes it.