Added utilities for saving and loading RasCAL2 files....#431
Added utilities for saving and loading RasCAL2 files....#431arwelHughes wants to merge 4 commits intoRascalSoftware:masterfrom
Conversation
utilities/misc/loadR2.m
Outdated
| problem = jsonToProject(projectFile); | ||
|
|
||
| % If there is a second argument, also load results.. | ||
| if nargout > 1 |
There was a problem hiding this comment.
@arwelHughes This does not convert controls.json is this intentional?
StephenNneji
force-pushed
the
saveR2
branch
2 times, most recently
from
cf6e22d to
366ddcb
Compare
There was a problem hiding this comment.
It not to for me to make suggestions on code quality at this stage of a project, but IMHO, cd within a function is a very bad practice. It is usually ok to do cd on top level tests ensuring onCleanup returns to intial folder in any case, but cd within a routine usually causes bunch of maintenance problem on a long run. to/from json insides look also a bit crude.
Code works and does the job. There are 2 bugs and 1 suggestion within the test suite itself.
| result2 = jsonToResults("test.json"); | ||
|
|
||
| props = properties(result); | ||
| for i = 1:length(props) | ||
| testCase.verifyEqual(result.(props{i}), result2.(props{i})); | ||
| end |
There was a problem hiding this comment.
request:
This may be previous contribution, but this test tests nothing. Structure contains no properties.
row 60 should be:
props = fieldnames(result);
And tests are failing if you do this. Seems on shape only but...
| for i = 1:length(props) | ||
| % verifies the problem name, model type and geometry |
There was a problem hiding this comment.
Request if I understand it correctly:
I am puzzled. Why this loop? Nothing inside it depends on i or props -- you just run the same check 16 times. Should loop be removed?
All prop check is correctly perfromed at rows 120-124.
Test between for and end partially repeats 120-124 but ok, this way of testing may be more beneficial or clear, but why to run it in a loop?
| evalc(scriptName); | ||
|
|
||
| controls = controlsClass(); | ||
| [~, project, result] = evalc('RAT(problem, controls);'); |
There was a problem hiding this comment.
Suggestion:
I can vageuly understand and accept why one would use evalc in 85, though IMHO function handle would be much better. But in 88! Why?
Why to use:
[~, project, result] = evalc('RAT(problem, controls);');
instead of just:
[project, result] = RAT(problem, controls);
It works faster and much more clear code.
4 participants
No description provided.