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.

Sign up for GitHub

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

Jump to bottom

BugFix: MsgSpatial interaction radius correct when not a factor of en… #1160

Merged
merged 3 commits into from
Dec 15, 2023
Merged

BugFix: MsgSpatial interaction radius correct when not a factor of en… #1160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
678b923
BugFix: MsgSpatial interaction radius correct when not a factor of en…
Robadob Dec 12, 2023
346bd67
Test that bug has been fixed.
Robadob Dec 12, 2023
3241d11
Seatbelts now prevents wrapped spatial iterator if radius is not a fa…
Robadob Dec 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions include/flamegpu/runtime/messaging/MessageSpatial2D/MessageSpatial2DDevice.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,13 @@ class MessageSpatial2D::In {
// Return iterator at min corner of env, this should be safe
return WrapFilter(metadata, metadata->min[0], metadata->min[1]);
}
if (fmodf(metadata->max[0] - metadata->min[0], metadata->radius) > 0.00001f ||
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason/justification for this particular epsilon value?.

Though our current spatial comms is fp32 only anyway, so a magic value is fine, just why this one / is it sufficient for different values

E.g. if the comm rad is a small positive number, will this work? (though if its too small then large env widths will cause other problems anyway, so can prolly get away with a fixed arbitrary epsilon)

Tests only appear to cover a single radii/width combo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any particular reason/justification for this particular epsilon value?

Nope, as mentioned in OP...

Happy for someone to update my fmod() solution to something cleaner, floating point isn't great.

Copy link
Member Author

Choose a reason for hiding this comment

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

(I'd rather make wrapped work at a tiny cost to perf for non-wrapped to avoid this floating point crap)

Copy link
Member

Choose a reason for hiding this comment

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

I agree that hurting wrapped more and not hurting non-wrapped is the better choice.

Given the radii and width are known on the host, we could maybe do the check there once, and then set a value on the device somewhere marking if wrapped is possioble for that message list or not? for less perf loss to wrapped usage. An extra global memory read is probably cheaper than a couple of fmodf's?

Could do it entirley on the host if we made wrapped opt-in on the message list, but I assume that's not desirable?

Not sure there's a better option than fmodf, jsut the particular epsilon might need some thought / broader testing.

fmodf(metadata->max[1] - metadata->min[1], metadata->radius) > 0.00001f) {
DTHROW("Spatial messaging radius (%g) is not a factor of environment dimensions (%g, %g),"
" this is unsupported for the wrapped iterator, MessageSpatial2D::In::wrap().\n", metadata->radius,
metadata->max[0] - metadata->min[0],
metadata->max[1] - metadata->min[1]);
}
#endif
return WrapFilter(metadata, x, y);
}
Expand Down Expand Up @@ -599,8 +606,8 @@ T MessageSpatial2D::In::WrapFilter::Message::getVariable(const char(&variable_na
__device__ __forceinline__ MessageSpatial2D::GridPos2D getGridPosition2D(const MessageSpatial2D::MetaData *md, float x, float y) {
// Clamp each grid coord to 0<=x<dim
int gridPos[2] = {
static_cast<int>(floorf(((x-md->min[0]) / md->environmentWidth[0])*md->gridDim[0])),
static_cast<int>(floorf(((y-md->min[1]) / md->environmentWidth[1])*md->gridDim[1]))
static_cast<int>(floorf((x-md->min[0]) / md->radius)),
static_cast<int>(floorf((y-md->min[1]) / md->radius))
};
MessageSpatial2D::GridPos2D rtn = {
gridPos[0] < 0 ? 0 : (gridPos[0] >= static_cast<int>(md->gridDim[0]) ? static_cast<int>(md->gridDim[0]) - 1 : gridPos[0]),
Expand Down
15 changes: 12 additions & 3 deletions include/flamegpu/runtime/messaging/MessageSpatial3D/MessageSpatial3DDevice.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,15 @@ class MessageSpatial3D::In {
// Return iterator at min corner of env, this should be safe
return WrapFilter(metadata, metadata->min[0], metadata->min[1], metadata->min[2]);
}
if (fmodf(metadata->max[0] - metadata->min[0], metadata->radius) > 0.00001f ||
fmodf(metadata->max[1] - metadata->min[1], metadata->radius) > 0.00001f ||
fmodf(metadata->max[2] - metadata->min[2], metadata->radius) > 0.00001f) {
DTHROW("Spatial messaging radius (%g) is not a factor of environment dimensions (%g, %g, %g),"
" this is unsupported for the wrapped iterator, MessageSpatial3D::In::wrap().\n", metadata->radius,
metadata->max[0] - metadata->min[0],
metadata->max[1] - metadata->min[1],
metadata->max[2] - metadata->min[2]);
}
#endif
return WrapFilter(metadata, x, y, z);
}
Expand Down Expand Up @@ -639,9 +648,9 @@ T MessageSpatial3D::In::WrapFilter::Message::getVariable(const char(&variable_na
__device__ __forceinline__ MessageSpatial3D::GridPos3D getGridPosition3D(const MessageSpatial3D::MetaData *md, float x, float y, float z) {
// Clamp each grid coord to 0<=x<dim
int gridPos[3] = {
static_cast<int>(floorf(((x-md->min[0]) / md->environmentWidth[0])*md->gridDim[0])),
static_cast<int>(floorf(((y-md->min[1]) / md->environmentWidth[1])*md->gridDim[1])),
static_cast<int>(floorf(((z-md->min[2]) / md->environmentWidth[2])*md->gridDim[2]))
static_cast<int>(floorf((x-md->min[0]) / md->radius)),
static_cast<int>(floorf((y-md->min[1]) / md->radius)),
static_cast<int>(floorf((z-md->min[2]) / md->radius))
};
MessageSpatial3D::GridPos3D rtn = {
gridPos[0] < 0 ? 0 : (gridPos[0] >= static_cast<int>(md->gridDim[0]) ? static_cast<int>(md->gridDim[0]) - 1 : gridPos[0]),
Expand Down
119 changes: 119 additions & 0 deletions tests/test_cases/runtime/messaging/test_spatial_2d.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,52 @@ TEST(Spatial2DMessageTest, Wrapped7) {
TEST(Spatial2DMessageTest, Wrapped_OutOfBounds) {
EXPECT_THROW(wrapped_2d_test(141.0f, -540.0f, 200.0f), exception::DeviceError);
}
FLAMEGPU_AGENT_FUNCTION(in_wrapped_EnvDimsNotFactor, MessageSpatial2D, MessageNone) {
const float x1 = FLAMEGPU->getVariable<float>("x");
const float y1 = FLAMEGPU->getVariable<float>("y");
for (auto& t : FLAMEGPU->message_in.wrap(x1, y1)) {
// Do nothing, it should throw a device exception
}
return ALIVE;
}
TEST(Spatial2DMessageTest, Wrapped_EnvDimsNotFactor) {
// This tests that bug #1157 is fixed
// When the interaction radius is not a factor of the width
// that agent's near the max env bound all have the full interaction radius
ModelDescription m("model");
MessageSpatial2D::Description message = m.newMessage<MessageSpatial2D>("location");
message.setMin(0, 0);
message.setMax(50.1f, 50.1f);
message.setRadius(10);
message.newVariable<flamegpu::id_t>("id"); // unused by current test
AgentDescription agent = m.newAgent("agent");
agent.newVariable<float>("x");
agent.newVariable<float>("y");
AgentFunctionDescription fo = agent.newFunction("out", out_mandatory2D);
fo.setMessageOutput(message);
AgentFunctionDescription fi = agent.newFunction("in", in_wrapped_EnvDimsNotFactor);
fi.setMessageInput(message);
LayerDescription lo = m.newLayer();
lo.addAgentFunction(fo);
LayerDescription li = m.newLayer();
li.addAgentFunction(fi);
// Set pop in model
CUDASimulation c(m);
// Create an agent in the middle of each edge
AgentVector population(agent, 1);
// Initialise agents
// Vertical pair that can interact
// Top side
AgentVector::Agent i1 = population[0];
i1.setVariable<float>("x", 25.0f);
i1.setVariable<float>("y", 25.0f);
c.setPopulationData(population);
c.SimulationConfig().steps = 1;
EXPECT_THROW(c.simulate(), exception::DeviceError);
}
#else
TEST(Spatial2DMessageTest, DISABLED_Wrapped_OutOfBounds) { }
TEST(Spatial2DMessageTest, DISABLED_Wrapped_EnvDimsNotFactor) { }
#endif
FLAMEGPU_AGENT_FUNCTION(out_mandatory2D_OddStep, MessageNone, MessageSpatial2D) {
if (FLAMEGPU->getStepCounter() % 2 == 0) {
Expand Down Expand Up @@ -964,5 +1008,80 @@ TEST(Spatial2DMessageTest, buffer_not_init) {
EXPECT_NO_THROW(c.simulate());
}

FLAMEGPU_AGENT_FUNCTION(in_bounds_not_factor, MessageSpatial2D, MessageNone) {
const float x1 = FLAMEGPU->getVariable<float>("x");
const float y1 = FLAMEGPU->getVariable<float>("y");
unsigned int count = 0;
// Count how many messages we received (including our own)
for (const auto& message : FLAMEGPU->message_in(x1, y1)) {
++count;
}
FLAMEGPU->setVariable<unsigned int>("count", count);
return ALIVE;
}
TEST(Spatial2DMessageTest, bounds_not_factor_radius) {
// This tests that bug #1157 is fixed
// When the interaction radius is not a factor of the width
// that agent's near the max env bound all have the full interaction radius
ModelDescription m("model");
MessageSpatial2D::Description message = m.newMessage<MessageSpatial2D>("location");
message.setMin(0, 0);
message.setMax(50.1f, 50.1f);
message.setRadius(10);
// Grid will be 6x6
// 6th column/row should only be 0.1 wide of the environment
// Bug would incorrectly divide the whole environment by 6
// So bin widths would instead become 8.35 (down from 10)
message.newVariable<flamegpu::id_t>("id"); // unused by current test
AgentDescription agent = m.newAgent("agent");
agent.newVariable<float>("x");
agent.newVariable<float>("y");
agent.newVariable<unsigned int>("count", 0);
AgentFunctionDescription fo = agent.newFunction("out", out_mandatory2D);
fo.setMessageOutput(message);
AgentFunctionDescription fi = agent.newFunction("in", in_bounds_not_factor);
fi.setMessageInput(message);
LayerDescription lo = m.newLayer();
lo.addAgentFunction(fo);
LayerDescription li = m.newLayer();
li.addAgentFunction(fi);
// Set pop in model
CUDASimulation c(m);
// Create an agent in the middle of each edge
AgentVector population(agent, 4);
// Initialise agents
// Vertical pair that can interact
// Top side
AgentVector::Agent i1 = population[0];
i1.setVariable<float>("x", 10.0f);
i1.setVariable<float>("y", 0.0f);
// Top side inner
AgentVector::Agent i2 = population[1];
i2.setVariable<float>("x", 10.0f);
i2.setVariable<float>("y", 18.0f);
// Right side
AgentVector::Agent i3 = population[2];
i3.setVariable<float>("x", 50.1f);
i3.setVariable<float>("y", 40.0f);
// Horizontal pair that can interact
// Right side inner
AgentVector::Agent i4 = population[3];
i4.setVariable<float>("x", 50.1f - 10.11f);
i4.setVariable<float>("y", 40.0f);
c.setPopulationData(population);
c.SimulationConfig().steps = 1;
EXPECT_NO_THROW(c.simulate());
// Recover the results and check they match what was expected
c.getPopulationData(population);
// Validate each agent has same result
for (AgentVector::Agent ai : population) {
if (ai.getID() < 3) {
EXPECT_EQ(2u, ai.getVariable<unsigned int>("count"));
} else {
EXPECT_EQ(1u, ai.getVariable<unsigned int>("count"));
}
}
}

} // namespace test_message_spatial2d
} // namespace flamegpu
139 changes: 139 additions & 0 deletions tests/test_cases/runtime/messaging/test_spatial_3d.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -976,8 +976,55 @@ TEST(Spatial3DMessageTest, Wrapped3) {
TEST(Spatial3DMessageTest, Wrapped_OutOfBounds) {
EXPECT_THROW(wrapped_3d_test(141.0f, -540.0f, 0.0f, 200.0f), exception::DeviceError);
}
FLAMEGPU_AGENT_FUNCTION(in_wrapped_EnvDimsNotFactor, MessageSpatial3D, MessageNone) {
const float x1 = FLAMEGPU->getVariable<float>("x");
const float y1 = FLAMEGPU->getVariable<float>("y");
const float z1 = FLAMEGPU->getVariable<float>("z");
for (auto &t : FLAMEGPU->message_in.wrap(x1, y1, z1)) {
// Do nothing, it should throw a device exception
}
return ALIVE;
}
TEST(Spatial3DMessageTest, Wrapped_EnvDimsNotFactor) {
// This tests that bug #1157 is fixed
// When the interaction radius is not a factor of the width
// that agent's near the max env bound all have the full interaction radius
ModelDescription m("model");
MessageSpatial3D::Description message = m.newMessage<MessageSpatial3D>("location");
message.setMin(0, 0, 0);
message.setMax(50.1f, 50.1f, 50.1f);
message.setRadius(10);
message.newVariable<flamegpu::id_t>("id"); // unused by current test
AgentDescription agent = m.newAgent("agent");
agent.newVariable<float>("x");
agent.newVariable<float>("y");
agent.newVariable<float>("z");
AgentFunctionDescription fo = agent.newFunction("out", out_mandatory3D);
fo.setMessageOutput(message);
AgentFunctionDescription fi = agent.newFunction("in", in_wrapped_EnvDimsNotFactor);
fi.setMessageInput(message);
LayerDescription lo = m.newLayer();
lo.addAgentFunction(fo);
LayerDescription li = m.newLayer();
li.addAgentFunction(fi);
// Set pop in model
CUDASimulation c(m);
// Create an agent in the middle of each edge
AgentVector population(agent, 1);
// Initialise agents
// Vertical pair that can interact
// Top side
AgentVector::Agent i1 = population[0];
i1.setVariable<float>("x", 25.0f);
i1.setVariable<float>("y", 25.0f);
i1.setVariable<float>("z", 25.0f);
c.setPopulationData(population);
c.SimulationConfig().steps = 1;
EXPECT_THROW(c.simulate(), exception::DeviceError);
}
#else
TEST(Spatial3DMessageTest, DISABLED_Wrapped_OutOfBounds) { }
TEST(Spatial3DMessageTest, DISABLED_Wrapped_EnvDimsNotFactor) { }
#endif
FLAMEGPU_AGENT_FUNCTION(out_mandatory3D_OddStep, MessageNone, MessageSpatial3D) {
if (FLAMEGPU->getStepCounter() % 2 == 0) {
Expand Down Expand Up @@ -1034,5 +1081,97 @@ TEST(Spatial3DMessageTest, buffer_not_init) {
c.SimulationConfig().steps = 4;
EXPECT_NO_THROW(c.simulate());
}

FLAMEGPU_AGENT_FUNCTION(in_bounds_not_factor, MessageSpatial3D, MessageNone) {
const float x1 = FLAMEGPU->getVariable<float>("x");
const float y1 = FLAMEGPU->getVariable<float>("y");
const float z1 = FLAMEGPU->getVariable<float>("z");
unsigned int count = 0;
// Count how many messages we received (including our own)
for (const auto& message : FLAMEGPU->message_in(x1, y1, z1)) {
++count;
}
FLAMEGPU->setVariable<unsigned int>("count", count);
return ALIVE;
}
TEST(Spatial3DMessageTest, bounds_not_factor_radius) {
// This tests that bug #1157 is fixed
// When the interaction radius is not a factor of the width
// that agent's near the max env bound all have the full interaction radius
ModelDescription m("model");
MessageSpatial3D::Description message = m.newMessage<MessageSpatial3D>("location");
message.setMin(0, 0, 0);
message.setMax(50.1f, 50.1f, 50.1f);
message.setRadius(10);
// Grid will be 6x6
// 6th column/row should only be 0.1 wide of the environment
// Bug would incorrectly divide the whole environment by 6
// So bin widths would instead become 8.35 (down from 10)
message.newVariable<flamegpu::id_t>("id"); // unused by current test
AgentDescription agent = m.newAgent("agent");
agent.newVariable<float>("x");
agent.newVariable<float>("y");
agent.newVariable<float>("z");
agent.newVariable<unsigned int>("count", 0);
AgentFunctionDescription fo = agent.newFunction("out", out_mandatory3D);
fo.setMessageOutput(message);
AgentFunctionDescription fi = agent.newFunction("in", in_bounds_not_factor);
fi.setMessageInput(message);
LayerDescription lo = m.newLayer();
lo.addAgentFunction(fo);
LayerDescription li = m.newLayer();
li.addAgentFunction(fi);
// Set pop in model
CUDASimulation c(m);
// Create an agent in the middle of each edge
AgentVector population(agent, 6);
// Initialise agents
// Vertical pair that can interact
// Top side
AgentVector::Agent i1 = population[0];
i1.setVariable<float>("x", 50.1f / 2);
i1.setVariable<float>("y", 0.0f);
i1.setVariable<float>("z", 50.1f / 2);
// Top side inner
AgentVector::Agent i2 = population[1];
i2.setVariable<float>("x", 50.1f / 2);
i2.setVariable<float>("y", 18.0f);
i2.setVariable<float>("z", 50.1f / 2);
// Right side
AgentVector::Agent i3 = population[2];
i3.setVariable<float>("x", 0.0f);
i3.setVariable<float>("y", 50.1f / 2);
i3.setVariable<float>("z", 1.0f / 2);
// Horizontal pair that can interact
// Right side inner
AgentVector::Agent i4 = population[3];
i4.setVariable<float>("x", 18.0f);
i4.setVariable<float>("y", 50.1f / 2);
i4.setVariable<float>("z", 1.0f / 2);
// Rear side
AgentVector::Agent i5 = population[4];
i5.setVariable<float>("x", 50.1f / 2);
i5.setVariable<float>("y", 50.0f);
i5.setVariable<float>("z", 50.1f);
// Horizontal pair that can interact
// Rear side inner
AgentVector::Agent i6 = population[5];
i6.setVariable<float>("x", 50.1f / 2);
i6.setVariable<float>("y", 50.0f);
i6.setVariable<float>("z", 50.1f - 10.11f);
c.setPopulationData(population);
c.SimulationConfig().steps = 1;
EXPECT_NO_THROW(c.simulate());
// Recover the results and check they match what was expected
c.getPopulationData(population);
// Validate each agent has same result
for (AgentVector::Agent ai : population) {
if (ai.getID() < 5) {
EXPECT_EQ(2u, ai.getVariable<unsigned int>("count"));
} else {
EXPECT_EQ(1u, ai.getVariable<unsigned int>("count"));
}
}
}
} // namespace test_message_spatial3d
} // namespace flamegpu