Conversation
…ng (#65, #66, #67) Three failures that named nothing useful. A wrong parameter type threw a bare std::bad_any_cast out of get_parameter(): no parameter, no material, no type, and not a class a caller would think to catch. Writing 1 where 1.0 is read -- an int for a double, which both JSON and C++ make easy -- produced "bad any_cast" and nothing else, at construction, before the material could say anything about itself. set_parameter() already wrapped this; get, which every material calls several times, did not. The JSON reader registry is keyed on std::type_index alone, and block_ref is std::pair<std::string, std::string>, so vector_newton's "zero_blocks" and weighted_sum's "terms" are the SAME C++ type and share one reader. A malformed weighted_sum term therefore reported zero_blocks: each entry must be a [row, column] pair naming a parameter of a different material. Readers now receive the parameter key -- it was already available at the read() call site and thrown away -- so the shared reader names whatever it was actually given, and add_for_key() lets one parameter keep its own wording without disturbing the other user of the type. Any future pair of parameters sharing a C++ type would have hit the same thing. A missing "type" surfaced nlohmann's own "key 'type' not found", from a library the user never named, with no indication of which entry was at fault. A misspelt one echoed the name back with no candidates. Both now name the entry by its "name", and an unknown type suggests the nearest registered one or lists them all when nothing is close. Not fixed here: a misspelt OPTIONAL key is still ignored silently in the C++ path. parameter_handler exposes no way to enumerate its keys, only find() and print(), so comparing a handler against a schema needs a numsim-core change. #65 stays open for that half. 310/310. Mutation tested: each check reverted in turn fails only its own tests. get_parameter is overloaded on how the key arrives and materials use both -- a string literal binds to the rvalue overload, a stored std::string to the const-reference one -- and the first version of the test reached only one, leaving the other's guard free to delete with the suite still green.
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.
Closes #66, #67. Partially addresses #65 (see below).
Three failures that named nothing useful.
A wrong parameter type was a bare
bad_any_cast(#65)get_parameter()letstd::bad_any_castescape: no parameter, no material, no type, and not a class a caller would think to catch. Writing1where1.0is read -- an int for a double, which both JSON and C++ make easy -- producedbad any_castand nothing else, at construction, before the material could say anything about itself.set_parameter()already wrapped this.get, which every material calls several times, did not.One reader served two different parameters (#66)
The JSON reader registry is keyed on
std::type_indexalone.block_refisstd::pair<std::string, std::string>, sovector_newton'szero_blocksandweighted_sum'stermsare the same C++ type and resolve to the same reader. A malformedweighted_sumterm reportednaming a parameter of a different material.
Readers now receive the parameter key -- it was already available at the
read()call site and thrown away -- so the shared reader names whatever it was actually given.add_for_key()lets one parameter keep its own wording without disturbing the other user of the type, sozero_blocksstill says[row, column].Any future pair of parameters sharing a C++ type would have hit the same thing.
"type"errors were a dead end (#67)A missing
"type"surfaced nlohmann's ownkey 'type' not found, from a library the user never named, with no indication of which entry was at fault. A misspelt one echoed the name back with no candidates.Both now name the entry by its
"name", and an unknown type suggests the nearest registered one (j2_plasticty->j2_plasticity) or lists them all when nothing is close.Not fixed here
#65 also covers a misspelt optional key being ignored silently in the C++ path.
parameter_handlerexposes no way to enumerate its keys -- onlyfind()andprint()-- so comparing a handler against a schema needs a numsim-core change. I have left #65 open for that half rather than closing it.Tests
tests/test_parameter_diagnostics.cpp, 10 tests. 310/310.get_parameterconst-ref overload unwrappedBothGetParameterOverloadsReportTheSameWayget_parameterrvalue overload unwrappedAMalformedWeightedSumTermDoesNotMentionZeroBlocks"type"check removedAMissingTypeKeyDoesNotLeakTheJsonLibraryget_parameteris overloaded on how the key arrives and materials use both -- a string literal binds to the rvalue overload, a storedstd::stringto the const-reference one. My first version of the test reached only one, so deleting the other's guard left the whole suite green. That survived mutant is the only reason I found it; the test now drives both.