SpineML / SpineML_2_BRAHMS

Code generation scripts to run a SpineML neural model using BRAHMS on standard CPUs
http://spineml.github.io/simulators/BRAHMS/
4 stars 3 forks source link

SpineML_2_BRAHMS does not honour delays in generic input connections. #30

Closed sebjameswml closed 7 years ago

sebjameswml commented 8 years ago

As per subject. SpineCreator permits the specification of delays, but SpineML_2_BRAHMS doesn't currently handle these.

sebjameswml commented 7 years ago

It appears that delays ARE implemented at least for some connection types. I'm working through the code to verify if this ticket can be closed.

However, I have a question for @ajc158 - Alex, there's code in SpineML_GenericInput_NL.xsl like this, for a OneToOneConnection:

                    <xsl:if test="count(.//SMLNL:OneToOneConnection)=1">
                        <!-- Link for OneToOneConnection -->
                        <Link>
                            <Src><xsl:value-of select="translate(@src,' -', '_H')"/><xsl:text disable-output-escaping="no">&gt;</xsl:text><xsl:value-of select="@src_port"/></Src>
                            <Dst><xsl:value-of select="translate(../@name,' -', '_H')"/><xsl:if test="count(document(../@url)//SMLCL:AnalogReducePort[@name=$dstPortRef])=1 or count(document(../@url)//SMLCL:EventReceivePort[@name=$dstPortRef]) or count(document(../@url)//SMLCL:ImpulseReceivePort[@name=$dstPortRef])"><xsl:text disable-output-escaping="no">&lt;</xsl:text></xsl:if><xsl:text disable-output-escaping="no">&lt;</xsl:text><xsl:value-of select="@dst_port"/></Dst>

                            <Lag>
                                <xsl:if test="count(.//SMLNL:FixedValue)=1 and not(number(.//SMLNL:FixedValue/@value)=0)">
                                    <xsl:value-of select="number(.//SMLNL:FixedValue/@value) div $expt_root//@dt"/>
                                </xsl:if>
                                <xsl:if test="not(count(.//SMLNL:FixedValue)=1) or number(.//SMLNL:FixedValue/@value)=0">1</xsl:if><!-- Why is this 1 timestep not 0? -->
                            </Lag>
                        </Link>
                    </xsl:if>

See the comment about the timestep not being 0 if the FixedValue's value is 0. What's the reason? If the timestep is really 0, then you could easily get into loops that BRAHMS can't compute, but then it informs you of this and won't run. Is it a performance optimisation thing?

ajc158 commented 7 years ago

That was implemented before the useful error reporting was implemented - and in those days a user would get a lot of fail without good explanation. I think we can go to zero delays now, but it will mean more errors for users.

On 15 November 2016 at 10:44, Seb James notifications@github.com wrote:

It appears that delays ARE implemented at least for some connection types. I'm working through the code to verify if this ticket can be closed.

However, I have a question for @ajc158 https://github.com/ajc158 - Alex, there's code in SpineML_GenericInput_NL.xsl like this, for a OneToOneConnection:

                <xsl:if test="count(.//SMLNL:OneToOneConnection)=1">
                    <!-- Link for OneToOneConnection -->
                    <Link>
                        <Src><xsl:value-of select="translate(@src,' -', '_H')"/><xsl:text disable-output-escaping="no">&gt;</xsl:text><xsl:value-of select="@src_port"/></Src>
                        <Dst><xsl:value-of select="translate(../@name,' -', '_H')"/><xsl:if test="count(document(../@url)//SMLCL:AnalogReducePort[@name=$dstPortRef])=1 or count(document(../@url)//SMLCL:EventReceivePort[@name=$dstPortRef]) or count(document(../@url)//SMLCL:ImpulseReceivePort[@name=$dstPortRef])"><xsl:text disable-output-escaping="no">&lt;</xsl:text></xsl:if><xsl:text disable-output-escaping="no">&lt;</xsl:text><xsl:value-of select="@dst_port"/></Dst>

                        <Lag>
                            <xsl:if test="count(.//SMLNL:FixedValue)=1 and not(number(.//SMLNL:FixedValue/@value)=0)">
                                <xsl:value-of select="number(.//SMLNL:FixedValue/@value) div $expt_root//@dt"/>
                            </xsl:if>
                            <xsl:if test="not(count(.//SMLNL:FixedValue)=1) or number(.//SMLNL:FixedValue/@value)=0">1</xsl:if><!-- Why is this 1 timestep not 0? -->
                        </Lag>
                    </Link>
                </xsl:if>

See the comment about the timestep not being 0 if the FixedValue's value is 0. What's the reason? If the timestep is really 0, then you could easily get into loops that BRAHMS can't compute, but then it informs you of this and won't run. Is it a performance optimisation thing?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SpineML/SpineML_2_BRAHMS/issues/30#issuecomment-260608550, or mute the thread https://github.com/notifications/unsubscribe-auth/AFid1qmwyRDdEwb97YUfn_5o3GAGsvSQks5q-YzxgaJpZM4JEME3 .

Alex Cope Research Associate Behavioural and Evolutionary Theory Lab Department of Computer Science, University of Sheffield www.alexcope.co.uk

sebjameswml commented 7 years ago

Ok, I'll set them to 0 as I work through all this delay stuff.

ajc158 commented 7 years ago

Just a note - but we need to establish safety checking on these changes - we can easily introduce bugs that would break existing models without care, and could use a way of testing this :-/

On 15 November 2016 at 10:58, Seb James notifications@github.com wrote:

Ok, I'll set them to 0 as I work through all this delay stuff.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SpineML/SpineML_2_BRAHMS/issues/30#issuecomment-260611335, or mute the thread https://github.com/notifications/unsubscribe-auth/AFid1iSM6owTtXLJ5Anj0s9A5KaSilBFks5q-ZA6gaJpZM4JEME3 .

Alex Cope Research Associate Behavioural and Evolutionary Theory Lab Department of Computer Science, University of Sheffield www.alexcope.co.uk

sebjameswml commented 7 years ago

The delays which aren't honoured for generic inputs are ConnectionLists. There IS a warning message in the brahms output, from this code in SpineML_GenericInput_NL.xsl:

<xsl:if test="count(.//SMLNL:FixedValue)=0">
    <xsl:message terminate="yes">Only Fixed delays for generic inputs are currently supported for BRAHMS</xsl:message>
</xsl:if>

But this doesn't seem to halt the simulator so the user won't necessarily notice.

ajc158 commented 7 years ago

If this doesn't halt the simulator we have a problem, as it will halt the xslt generation - we therefore need to check the return value on this bit of xslt generation for errors and end if they occur

On 16 Nov 2016 10:34 a.m., "Seb James" notifications@github.com wrote:

The delays which aren't honoured for generic inputs are ConnectionLists. There IS a warning message in the brahms output, from this code in SpineML_GenericInput_NL.xsl:

Only Fixed delays for generic inputs are currently supported for BRAHMS/xsl:message /xsl:if But this doesn't seem to halt the simulator so the user won't necessarily notice. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SpineML/SpineML_2_BRAHMS/issues/30#issuecomment-260911612, or mute the thread https://github.com/notifications/unsubscribe-auth/AFid1oZvj77UiHSVnuNTAEgmP61NwKE7ks5q-tw5gaJpZM4JEME3 .
sebjameswml commented 7 years ago

Indeed, it doesn't halt simulation - it halts the xslt resulting in a sys.xml file which contains only Processes and no Links, but which will in any case run.

I just tried one option which is to stick some dodgy XML into sys.xml to force BRAHMS to close with a useful message, but I'll look at the return value of the xsltproc as a better option.

sebjameswml commented 7 years ago

a6a087f ensures that on any error in the xsltproc calls, the script exits with an error message.

sebjameswml commented 7 years ago

Closing. Yes, it's true that for generic connections, SpineML_2_BRAHMS doesn't honour per-connection delays in connection lists, nor does it honour randomly distributed delays. However, it does now prevent the execution of models which have such specifications.

sebjameswml commented 7 years ago

Just a note on the connection lag. If you have a ConnectionList, in which all the delays are specified, which participates in a loop, BRAHMS doesn't know about the delays that are going to be handled in the WU component, and so if the Lag in the SystemML Link is set to 0, BRAHMS complains about an impossible loop. So, I want SpineML_2_BRAHMS to notice if there's a connection list and put the Lag in as 1 in that case. Could even subtract one timestep from the delays in the ConnectionList, but that might be just too confusing.

sebjameswml commented 7 years ago

803093e adds that timestep.

sebjameswml commented 7 years ago

@ajc158 - take a look at SpineML_2_BRAHMS and SpineML_Preflight in the expt_delay_change_option and expt_delay_change branches (respectively). You may find that any models you have with generic connections that have explicit lists will no longer work at all. Previously, the code which was supposed to terminate the xsltproc wasn't in fact stopping it. The xml:message had terminat="yes" - no 'e' on the terminate and also the script wasn't checking the return value of xsltproc.

Anyway, now it is doing the right thing, it seems that we can't have any generic connections with explicit lists, so there's a bit more work to do, so that we can have explicit lists along with fixed delays for generic connections.

ajc158 commented 7 years ago

Generic connections with explicit lists do work... Basically all my models have it and if it didn't work then they wouldn't! Maybe that terminate is obsolete?

On 16 Nov 2016 11:27 p.m., "Seb James" notifications@github.com wrote:

@ajc158 https://github.com/ajc158 - take a look at SpineML_2_BRAHMS and SpineML_Preflight in the expt_delay_change_option and expt_delay_change branches (respectively). You may find that any models you have with generic connections that have explicit lists will no longer work at all. Previously, the code which was supposed to terminate the xsltproc wasn't in fact stopping it. The xml:message had terminat="yes" - no 'e' on the terminate and also the script wasn't checking the return value of xsltproc.

Anyway, now it is doing the right thing, it seems that we can't have any generic connections with explicit lists, so there's a bit more work to do, so that we can have explicit lists along with fixed delays for generic connections.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SpineML/SpineML_2_BRAHMS/issues/30#issuecomment-261106242, or mute the thread https://github.com/notifications/unsubscribe-auth/AFid1m3_7n9v5r-7k_GTok2WjFCdLMMmks5q-5FdgaJpZM4JEME3 .

sebjameswml commented 7 years ago

The existing code does implement the connection via the explicit list, but I don't know what would've happened to the connection delays, I think a Lag of 1 timestep is always set, so that wouldn't necessarily be what the user expected. Have fixed this in those branches I mentioned, am working to implement the ability to specify a global delay in generic connections in SpineCreator

sebjameswml commented 7 years ago

Have you time for a quick meet up next week to discuss this - I can show you the changes I've made in the various new branches?

sebjameswml commented 7 years ago

Closing this ticket, because a) Current SpineML_2_BRAHMS does the right thing, that is it tells the user that it can't proceed if there are connection lists with per-connection delays and b) This limitation may be removed if I finish #20