Conversation
Parses arguments with "key=value" form as keywords, so
dirichlet(1, width=3)
passes into `clone` a list of arguments ["1"] and a map of keywords
{"width":"3"}. This is not particularly clever, so may go wrong in a
variety of interesting ways. It should work for the simple cases we
support.
Unit test of BoundaryFactory registers a dummy boundary operation,
then uses this to test different combinations of positional arguments
and keywords.
Fixed long-standing bug in boundary factory, where this was intended
to slice from start to i, but actually produced a string of length i.
string s = arg.substr(start, i);
Contributor
|
I don't have a strong opinion. I'm merging into BoundaryOp-refactor at the moment, but I'd lean to merging this PR into next already since it has a bugfix and a test. |
Member
|
Should the bug fix strictly go into master? If it's lived in the code for a long time then I guess it's probably not causing any problems. |
ZedThree
reviewed
Nov 2, 2018
* next: (83 commits)
Improve some comments; use uniform initialisation more consistently
Default initialise field sizes and Field::bndry* status
Default initialise boundaryIs{Copy,Set} in FieldData
Default initialise some Field3D/2D members
Add move constructor for Field2D
Add swap function for Field2D
Add move constructor for Field3D
Fix:Escaping in makefile
Documentation improvements
Improve documentation
Replace tabs with spaces
Correct trace statement
Add missing TRACE call
boutcore docs update
Add documentation
Remove regeneration test
Print command line options to output_info
Remove ignored arguments from AUTO_TRACE and add doxygen comment
Save Coordinates* pointer in Laplacian
Remove usage of global 'mesh' pointer in Laplacian solvers
...
ZedThree
approved these changes
Nov 20, 2018
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
4 participants
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.
Not sure if this should go into
nextorBoundaryOp-refactorfirst... Contains a new feature (keywords for boundary operations), and a fix for a previously existing bug which was found while adding unit tests.