Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Step Type Query #91

Merged
merged 10 commits into from
Jan 27, 2025
Merged

Step Type Query #91

merged 10 commits into from
Jan 27, 2025

Conversation

coliem
Copy link

@coliem coliem commented Jan 23, 2025

Closes #65

  • Updated CMake to disable C# and added a new build Debug-C.
  • Implemented C++ methods to query and store all the step types of a given graph (accomplished using C++ functions CalculateStepType and 'CalculateAndStoreStepType' in graph_utils).
  • C_Interface function CalculateAndStoreStepTypes call the previously mentioned C++ methods, also storing all the step types in the given graph.
  • Python/C CalculateAndStoreStepTypes calls the new C_Interface function to query the step types in the Python interface.
  • Added UnitTests corresponding to each of these functions.
  • Created overload for CheckConnection in graph_utils that removes the params parameter. This is utilized in CalculateStepType and is allowed because we know that the edges are valid after graph generation.
  • Created CompareCheckConnection to test equality between the two overloads for CheckConnection. Once CalculateAndStoreStepType(s) is called, each edge step type found in the resulting std::vector<EdgeSet> is tested against the output of the corresponding step type given by CheckConnection with the params parameter. This function's purpose was to simplify the appropriate UnitTests.

Attached is a screenshot of all the VS test cases (including three new test cases for the C++ and C_Interface functions, with the exception of HFUnitTests/CInterfaceTests/C_Pathfinder/CreateManyPathsPerformanceTest due to runtime issues) and the test_GraphGenerator.py test cases passing.
image
image

src/CMakeSettings.json Show resolved Hide resolved
src/Cinterface/analysis_C.cpp Show resolved Hide resolved
src/Cpp/tests/src/GraphAlgorithms.cpp Outdated Show resolved Hide resolved
HF::GraphGenerator::GraphParams params;

//Setup params struct
params.up_step = up_step; params.down_step = down_step;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these are needed, since the edge validity already exists, we only need to check what type of edge it is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The step types aren't stored anywhere when the graph is generated, so I have to call

HF::SpatialStructures::STEP CheckConnection(
const real3& parent,
const real3& child,
RayTracer& rt,
const GraphParams& params)
{

which takes these parameters as input. The parameters are actively used in code blocks such as

if (parent[2] > child[2])
{
node1 = child;
node2 = parent;
node1[2] = node1[2] + params.down_step;
node2[2] = node2[2] + GROUND_OFFSET;
s = STEP::DOWN;
}

The workaround I had for this was using default values for these parameters corresponding the the default values in the GraphGenerator

    up_step: float = 0.197,
    up_slope: float = 20,
    down_step: float = 0.197,
    down_slope: float = 20,
    ground_offset: float = 0.01,
    node_z: float = 0.0001,
    node_spacing: float = 0.00001

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard my previous comment, I can remove the parameters.

Copy link
Author

@coliem coliem Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the unnecessary parameters and replaced the corresponding UnitTests with more complex ones. My initial PR is updated with any new functions I made.

@coliem coliem changed the title Steps Step Type Query Jan 25, 2025

def test_step_type_query():
mesh_path = dhart.get_sample_model("plane.obj")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plane is probably not the best mesh to test with since it has no steps

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to make the graph steps work with obstacle_vistestcase.obj

>>> CalculateAndStoreStepTypes(g, bvh)

>>> print(g.GetEdgeCost(0, 1, "step_type"))
3.0
Copy link
Owner

@cadop cadop Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the documentation needs to clarify what "3" means (i.e. the mapping)

@cadop cadop changed the base branch from main to dev January 27, 2025 21:35
@cadop cadop merged commit 4482e69 into cadop:dev Jan 27, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants