Closed Lance-Drane closed 1 year ago
Can one of the admins verify this patch?
OK to test
We now have a test failing (see here). I think this because, as you pointed out, the number of segments was irrelevant before this PR. Can you change the number of segments from 2 to 3 in tests/data/scan_path_L.txt
We now have a test failing (see here). I think this because, as you pointed out, the number of segments was irrelevant before this PR. Can you change the number of segments from 2 to 3 in tests/data/scan_path_L.txt
Happy to make this change. There's another test file, integration_da_add_material_sp.txt
, which will now only analyze the first 3 data lines instead of all of them. Would you like for me to change the number of path segments here to 13
instead of the initial value of 3
?
There are a couple of other files tests/data/thermoelastic_bare_plate_add_material_scan_path.txt
and tests/data/thermoelastic_bare_plate_scan_path.txt
which say that the number of path segments is 3
but there are only 2 lines of data; however, this shouldn't affect any tests, as the whole file will continue to be parsed. Should those files be updated, or are they fine as-is?
Yes, please update all the files.
Thanks for the fix!
Hi, I'm part of the INTERSECT SDK team - I help a little bit with integrating the adamantine digital twin into the overall INTERSECT deployment. Marshall McDonnell and Jesse McGaha can vouch for me. While debugging the deployment, Jesse and I noticed a few oddities with the segment file scanner:
&&
instead of||
. As currently implemented, the entire file will always be scanned, and the value ofn_segments
is never relevant. My change fixes this.&&
were implemented correctly,n_segments
was also subject to the off-by-one error. For example, ifnumber of path segments
was evaluated to1
, none of the path segments would have been executed.