Merge pull request #277 from Microsoft/exemsmelly

Changed interchange of rewards from xml to simple string format.

Malmo/src/AgentHost.cpp

@@ -407,7 +407,7 @@ namespace malmo
                     if( final_reward_optional.present() ) {
                         TimestampedReward final_reward(xml.timestamp,final_reward_optional.get());
                         this->processReceivedReward(final_reward);
-                        this->rewards_server->recordMessage(TimestampedString(xml.timestamp, final_reward.getAsXML(false)));
+                        this->rewards_server->recordMessage(TimestampedString(xml.timestamp, final_reward.getAsSimpleString()));
                     }
                 }
             }
@@ -492,14 +492,14 @@ namespace malmo
     void AgentHost::onReward(TimestampedString message)
     {
         boost::lock_guard<boost::mutex> scope_guard(this->world_state_mutex);
-
         try {
-            TimestampedReward reward(message.timestamp, message.text);
+            TimestampedReward reward;
+            reward.createFromSimpleString(message.timestamp, message.text);
             this->processReceivedReward(reward);
         }
-        catch( const xml_schema::exception& e ) {
+        catch (std::exception& e) {
             std::ostringstream oss;
-            oss << "Error parsing Reward message XML: " << e.what() << " : " << e << ":" << message.text.substr(0, 20) << "...";
+            oss << "Error parsing Reward message: " << e.what() << " : " << message.text;
             TimestampedString error_message(message);
             error_message.text = oss.str();
             this->world_state.errors.push_back(boost::make_shared<TimestampedString>(error_message));

Malmo/src/TimestampedReward.cpp

@@ -32,13 +32,19 @@
 
 namespace malmo
 {
+    TimestampedReward::TimestampedReward()
+    {
+    }
+
     TimestampedReward::TimestampedReward(float reward)
     {
         this->values[0] = static_cast<double>(reward);
     }
 
-    TimestampedReward::TimestampedReward(boost::posix_time::ptime timestamp,std::string xml_string)
+    TimestampedReward& TimestampedReward::createFromXML(boost::posix_time::ptime timestamp, std::string xml_string)
     {
+        this->timestamp = timestamp;
+
         const bool validate = true;
 
         xml_schema::properties props;
@@ -51,6 +57,32 @@ namespace malmo
         std::istringstream iss(xml_string);
         std::unique_ptr<malmo::schemas::Reward> reward = malmo::schemas::Reward_(iss, flags, props);
         setValuesFromRewardStructure(*reward);
+        return *this;
+    }
+
+    TimestampedReward& TimestampedReward::createFromSimpleString(boost::posix_time::ptime timestamp, std::string simple_string)
+    {
+        this->timestamp = timestamp;
+
+        // String should be comma-delimited sets of <dimension>:<value>.
+        size_t nextpos = 0, lastpos = 0;
+        while (nextpos != std::string::npos)
+        {
+            nextpos = simple_string.find(",", lastpos);
+            std::string token = (nextpos != std::string::npos) ? simple_string.substr(lastpos, nextpos - lastpos) : simple_string.substr(lastpos);
+            size_t split = token.find(":");
+            if (split == std::string::npos)
+            {
+                throw std::runtime_error("Malformed reward message.");
+            }
+            else
+            {
+                int dimension = std::stoi(token.substr(0, split));
+                double value = std::stod(token.substr(split + 1));
+                this->values[dimension] = value;
+            }
+            lastpos = nextpos + 1;
+        }
     }
 
     TimestampedReward::TimestampedReward(boost::posix_time::ptime timestamp,const schemas::Reward& reward)
@@ -94,6 +126,17 @@ namespace malmo
         return oss.str();
     }
 
+    std::string TimestampedReward::getAsSimpleString() const
+    {
+        std::ostringstream oss;
+        for (std::map<int, double>::const_iterator it = this->values.begin(); it != this->values.end(); it++) {
+            if (it != this->values.begin())
+                oss << ",";
+            oss << it->first << ":" << it->second;
+        }
+        return oss.str();
+    }
+
     bool TimestampedReward::hasValueOnDimension(int dimension) const
     {
         return this->values.find(dimension) != this->values.end();

Malmo/src/TimestampedReward.h

@@ -35,12 +35,18 @@ namespace malmo
     class TimestampedReward
     {
         public:
+            //! Constructs an empty reward.
+            TimestampedReward();
+
             //! Constructs from a single reward float (assumes default dimension of 0)
             TimestampedReward(float reward);
 
             //! Constructs from an XML string.
-            TimestampedReward(boost::posix_time::ptime timestamp,std::string xml_string);
-
+            TimestampedReward& createFromXML(boost::posix_time::ptime timestamp,std::string xml_string);
+
+            //! Constructs from a simple string.
+            TimestampedReward& createFromSimpleString(boost::posix_time::ptime timestamp, std::string simple_string);
+
             //! Constructs from an XML node element.
             TimestampedReward(boost::posix_time::ptime timestamp,const schemas::Reward& reward);
 
@@ -51,7 +57,11 @@ namespace malmo
             //! \param prettyPrint If true, add indentation and newlines to the XML to make it more readable.
             //! \returns The reward as an XML string.
             std::string getAsXML( bool prettyPrint ) const;
-
+
+            //! Formats as a simple string.
+            //! \returns The reward in simple string form.
+            std::string getAsSimpleString() const;
+
             //! The timestamp.
             boost::posix_time::ptime timestamp;
 

Minecraft/src/main/java/com/microsoft/Malmo/Client/ClientStateMachine.java

@@ -1495,7 +1495,8 @@ private void sendData()
                 currentMissionBehaviour().rewardProducer.getReward(currentMissionInit(), reward);
                 if (!reward.isEmpty())
                 {
-                    if (this.rewardSocket.sendTCPString(reward.getAsString()))
+                    String strReward = reward.getAsSimpleString();
+                    if (this.rewardSocket.sendTCPString(strReward))
                     {
                         this.failedTCPRewardSendCount = 0; // Reset the count of consecutive TCP failures.
                     }

Minecraft/src/main/java/com/microsoft/Malmo/MissionHandlers/MultidimensionalReward.java

@@ -97,7 +97,7 @@ public Reward getAsReward() {
      * 
      * @return the XML string.
      */
-    public String getAsString() {
+    public String getAsXMLString() {
         // Create a string XML representation:
         String rewardString = null;
         try {
@@ -108,6 +108,24 @@ public String getAsString() {
         return rewardString;
     }
 
+    /**
+     * Gets the reward structure as a simple, easily parsed string<br>
+     * Format: <dimension>:<value>, comma delimited.
+     * eg "0:45.6,1:32.2,12:-1.0" etc
+     * 
+     * @return the string.
+     */
+    public String getAsSimpleString() {
+        String rewardString = "";
+        for (Map.Entry<Integer, Float> entry : this.map.entrySet()) {
+            Integer dimension = entry.getKey();
+            Float reward_value = entry.getValue();
+            if (!rewardString.isEmpty())
+                rewardString += ",";
+            rewardString += dimension + ":" + reward_value;
+        }
+        return rewardString;
+    }
     /**
      * Resets the storage to empty.
      */

changelog.txt

@@ -1,5 +1,6 @@
 0.16.2
 -------------------
+Fix: [BREAKING CHANGE] Rewards now sent as simple strings rather than XML, for speed - changes recorded rewards format (#261)
 New: ALEAgentHost.setSeed allows ALE experiments to be seeded (#254)
 Fix: No longer need a fresh MissionRecordSpec for each call to startMission (#256)
 New: [BREAKING CHANGE] MissionRecordSpec.getTemporaryDirectory() now moved to AgentHost.getRecordingTemporaryDirectory()