Repository navigation
Support for values_indexed(model, nodes, i) - #72
Merged
Merged
Conversation
This file contains hidden or 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
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.
This provides support for
values(model, nodes[i])innimble2. Per discussion with @paciorek, we will follow this separation of syntax for now, wherenimbleandnimble2essentially support this by syntax sugar, and in nimbleModel we will instead support just the underlying feature needed.The cases for
values()turned out to be complicated. A third argument is added,mode= "raw", "scalar", or "node"."raw" means the i-th node is exactly the i-th element in nodes (better called "var subsets", which might be character, a list of varRanges, or an instrList). When
nodeis character,mode="raw"(the default) matches nimble, which should provide clean backward compatibility. "scalar" means to treat each scalar element as its own item, all consecutively indexed. This matches a nimble workflow where nodes were expanded into scalar elements, which we don't actually want to do in nimble2.mode = "node"means to create aninstrList(based on nodes) fromnodesand index index through that. This matches a nimble workflow where nodes were created as actual nodes (e.g., as ininitializeModel), all consecutively indexed, which we also don't actually want to do in nimble2.This branches from and would be merged back into branch
instr-indexing, the full set of support formodel$calculate(nodes[i])in nimble2 viamodel$calculate_indexed(nodes, i)and the related other model methods.Hence if tests pass, I will merge this, then I will go back to the PR for
instr-indexeing.