Refactor permission substitution to do all substitutions at once#436
Closed
cmacmackin wants to merge 74 commits into
Closed
Refactor permission substitution to do all substitutions at once#436cmacmackin wants to merge 74 commits into
cmacmackin wants to merge 74 commits into
Conversation
cmacmackin
force-pushed
the
cmacmackin/refactor_permission_substitution
branch
from
e3b61b2 to
43b78ec
Compare
cmacmackin
force-pushed
the
cmacmackin/access_control
branch
from
05b64e5 to
e302db2
Compare
I've specified that these tests are for the Braginskii closure. I've also renamed the existing collision tests accordingly. This will be useful when we add alternative closures in future. I've also caught an error in the existing collisions operator, which means it will sometimes assume that a species is charged if the `charge` variable has been set to 0 (rather than left unsent).
Unit tests have been updated and pass. I still need to update the regression tests.
Co-authored-by: Peter Hill <peter.hill@york.ac.uk>
All unit and integration tests pass, but many components are untested. There may be errors in their permissions.
Also added an additional constructor for SpeciesInfo
Co-authored-by: Peter Hill <peter.hill@york.ac.uk>
cmacmackin
force-pushed
the
cmacmackin/access_control
branch
from
e302db2 to
3b76f8f
Compare
cmacmackin
force-pushed
the
cmacmackin/refactor_permission_substitution
branch
from
43b78ec to
a2c4f80
Compare
cmacmackin
force-pushed
the
cmacmackin/access_control
branch
from
3b76f8f to
a79db98
Compare
Collaborator
Author
|
It was agreed that this didn't make sense as a change. |
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
1 participant
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 was a suggestion made by @ZedThree when review #421. I thought I saw a way to do this but it proved to be quite involved. By the time I realised that I was most of the way done so I decided to keep the work on a separate branch just in case we wanted to use it for something. However, I think this approach is actually less flexible and ultimately more complex. For example, it is now harder to predict the order in which substitutions occur. This means that if there are two substitutions that will ultimately result in the same variable name we don't know which one will be kept.