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

Agent's located outside of env bounds read from some bins twice during spatial partitioning. #13

Closed
Robadob opened this issue Jul 12, 2016 · 4 comments

Comments

@Robadob
Copy link
Member

Robadob commented Jul 12, 2016

If an agent is located outside of the environmental bounds min<= x <max their grid location is wrapped. Due to the wrapping only replacing out of bounds locations with the opposite bound, an agent that is out of bounds, can search further out of bounds cells, causing both cells to be 'wrapped' to the same cell.

Hence causing the agent to double dip.

Potential fixes:

  • Do proper modular wrapping
  • Throw an assertion error when agents are located out of bounds
  • Increase env max by +radius (and potentially decrease min by the same)
    • If you only increase env max by 1, the number of bins increases, however they are shared across the env width, meaning that the interaction radius is subsequently reduced.
@mondus
Copy link
Member

mondus commented Jul 13, 2016

Needs to have assertion errors to stop users from allowing agents to create messages outside of the bounds of the message environment. Will be integrated in version 2.

@Robadob
Copy link
Member Author

Robadob commented Aug 8, 2016

Snip
Neither XLST or C preprocessor macro's support (clean) floating point arithmetic, so it isn't easily feasible to check this is correct at compile time (I just hacked together a quick solution using pre-processor integer arithmetic, before noticing floating point values have been used within the examples).

Presumably such a check would therefore need to be included within the initialisation function of current FLAMEGPU, this is less desirable, but I don't see a solution for warning about this earlier. It might be possible to achieve this with XLSTs limited mathematical operations, however I'm unsure whether FLAMEGPUs packaged xlst preprocessor supports these and where I can find clear documentation on their usage.

@Robadob
Copy link
Member Author

Robadob commented Aug 8, 2016

Managed to get XLST math to work correctly with alot of trial and error (removing the // or space before - seemingly breaks this code), will test examples still build and commit.

<!--Compile time error if partitioning radius is not a factor of the partitioning dimensions as this causes partioning to execute incorrectly-->
<xsl:for-each select="gpu:xmodel/xmml:messages/gpu:message/gpu:partitioningSpatial">
<xsl:if test="(//gpu:xmax - //gpu:xmin) != (floor((//gpu:xmax - //gpu:xmin ) div //gpu:radius ) * //gpu:radius)">
#error "Partitioning radius must be a factor of partitioning dimensions.\nRadius: <xsl:value-of select="gpu:radius"/>, Xmin: <xsl:value-of select="gpu:xmin"/>, Xmax: <xsl:value-of select="gpu:xmax"/>"
</xsl:if><xsl:if test="(//gpu:ymax - //gpu:ymin) != (floor((//gpu:ymax - //gpu:ymin ) div //gpu:radius ) * //gpu:radius)">
#error "Partitioning radius must be a factor of partitioning dimensions.\nRadius: <xsl:value-of select="gpu:radius"/>, Ymin: <xsl:value-of select="gpu:ymin"/>, Ymax: <xsl:value-of select="gpu:ymax"/>"
</xsl:if><xsl:if test="(//gpu:zmax - //gpu:zmin) != (floor((//gpu:zmax - //gpu:zmin ) div //gpu:radius ) * //gpu:radius)">
#error "Partitioning radius must be a factor of partitioning dimensions.\nRadius: <xsl:value-of select="gpu:radius"/>, Zmin: <xsl:value-of select="gpu:zmin"/>, Zmax: <xsl:value-of select="gpu:zmax"/>"
</xsl:if>
</xsl:for-each>

@Robadob Robadob closed this as completed in a07955e Aug 8, 2016
@Robadob
Copy link
Member Author

Robadob commented Aug 9, 2016

Just updated the fix for this (and overwrote the prev fix). Discussion with Paul clarified that // notation searches from the root, meaning the above fix could work incorrectly where multiple spatial partitions exist.

I tested it again without the slashes, and the earlier error seems to have disappeared, so presumably I fixed whatever was causing that (potentially incorrect division, although that would mean it doesn't parse L2R) whilst arranging the rest of the algorithm.

82d7dc9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants