Adjustments to https://github.com/GabijaBern/raycloudtools/tree/new_command_line_params #24
Conversation
| @@ -29,7 +29,7 @@ bool readLas(const std::string &file_name, | |||
|
|
|||
| if (ifs.fail()) | |||
| { | |||
| std::cerr << "readLas: failed to open stream" << std::endl; | |||
| std::cerr << "readLas: failed to open file " << file_name << ", error: " << std::strerror(errno) << std::endl; | |||
There was a problem hiding this comment.
I am not certain this error will inform if file does not exist.
There was a problem hiding this comment.
It does. Just tested it.
"readLas: failed to open file doesnottexist.blh, error: No such file or directory"
| @@ -283,6 +285,10 @@ Trees::Trees(Cloud &cloud, const Eigen::Vector3d &offset, const Mesh &mesh, cons | |||
| removeOutOfBoundSections(cloud, min_bound, max_bound, offset); | |||
| } | |||
|
|
|||
| if (params_->radius_min > 0.0) | |||
There was a problem hiding this comment.
I am curious to understand the code a little bit better. If I run this approach, will it break the algorithm? What does bifurcate() method do?
There was a problem hiding this comment.
removeSmallRadiusTrees() is called towards the end when the radius() function will be accurate. We can't filter based on estimated_radius because this is not the final estimation, just an initial estimate.
bifurcate() is a function for splitting a segment at a tree branching point, so it adds to segments (cylinders) to the list, the key line being: sections_.push_back(new_node); Usually it is just a split in two, but it may also split into multiple branches.
|
Hey, I am actually taking another look at all of this, I had little time and pushed non-working code. I am going to take your ideas and revisit my PR. |
|
OK, let me know when you want to review, at the moment the PR contains a lot of additional stuff like output messages, RANSAC, thresholds etc that I wouldn't want to push into main.
If you're OK with the way I implemented the 3 features you needed then you can accept my PR and I'll merge it to main.
Thomas Lowe
Senior Experimental Scientist
Data61 | CSIRO
***@***.******@***.***> | 07 3327 4027 | 0487 433 606 (optional)
1 Technology Court, Pullenvale, QLD, 4069
CSIRO acknowledges the Traditional Owners of the land, sea and waters, of the area that we live and work on across Australia. We acknowledge their continuing connection to their culture and we pay our respects to their Elders past and present.
CSIRO Australia’s National Science Agency | csiro.au
…________________________________
From: GabijaBern ***@***.***>
Sent: Wednesday, 20 November 2024 11:12 PM
To: csiro-robotics/raycloudtools ***@***.***>
Cc: Lowe, Tom (Data61, Pullenvale) ***@***.***>; Author ***@***.***>
Subject: Re: [csiro-robotics/raycloudtools] Adjustments to https://github.com/GabijaBern/raycloudtools/tree/new_command_line_params (PR #24)
Hey, I am actually taking another look at all of this, I had little time and pushed non-working code. I am going to take your ideas and revisit my PR.
—
Reply to this email directly, view it on GitHub<#24 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKSVQ4FNCPBDLYKIZDB5SLL2BSDDLAVCNFSM6AAAAABR2C7JTSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBYGU2DQMZQHE>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
| Eigen::Vector2d mid_local(mid[0], mid[1]); | ||
| mid_local -= params_->grid_origin; | ||
| const Eigen::Vector2d inds(std::floor(mid_local[0] / width), std::floor(mid_local[1] / width)); | ||
| min_bound[0] = width * (inds[0]) + params_->grid_origin[0] - offset[0]; |
There was a problem hiding this comment.
If I am understanding your implementation correctly, you don't need to add the params_->grid_origin[0], for subsequent calls as well, otherwise, you remove the grid origin shift?
There was a problem hiding this comment.
Basically it is: grid_origin + snap_to_grid(mid - grid_origin)
which means snapping to an offset grid.
I've tested it on simple cases and seems to work.
| max_bound[0] = width * (inds[0] + 0.5) - offset[0]; | ||
| max_bound[1] = width * (inds[1] + 0.5) - offset[1]; | ||
| Eigen::Vector2d mid_local(mid[0], mid[1]); | ||
| mid_local -= params_->grid_origin; |
There was a problem hiding this comment.
And shouldn't this be mid_local +=?
There was a problem hiding this comment.
Basically it is: grid_origin + snap_to_grid(mid - grid_origin)
which means snapping to an offset grid.
I've tested it on simple cases and seems to work.
The '-d' treads on the distance_limit param, so I had to change it to -j in my PR. One reason why I like to minimise command line parameters.
The new_command_line_params changes are implemented a bit differently here, so this is instead of that branch.