Feature/506 set data buffer method in storage unit #507
Conversation
LorenzzoQM
force-pushed
the
feature/506_setDataBuffer_method_in_storageUnit
branch
from
b57d356 to
9b5dc30
Compare
LorenzzoQM
deleted the
feature/506_setDataBuffer_method_in_storageUnit
branch
| @@ -34,7 +34,7 @@ | |||
|
|
|||
| struct dataInstance { | |||
| char dataInstanceName[128]; //!< data instance name | |||
| double dataInstanceSum; //!< data instance sum value, bits | |||
| long long int dataInstanceSum; //!< data instance sum value, bits | |||
There was a problem hiding this comment.
I realize that this has already been merged, but maybe after the break, this can be changed to use the precise width type int64_t or even uint64_t if your semantics demand it. We should prefer precise width types for any integer types larger than int. The bit size of integral types is not guaranteed across platforms/compilers. It benefits us to er on the side of guarantee. You can see (for the large majority) this attempt throughout the code base to use int32_t, int64_t, and the unsigned versions.
There was a problem hiding this comment.
Howdy @LorenzzoQM , @patkenneally makes some good points here about the use of int64_t, etc. Could you please create a small branch next week to address his feedback? I'll respond to review request quickly so it doesn't slow down your research tasks.
There was a problem hiding this comment.
Sure! I will address the comments as soon as possible. Thank you for the feedback, @patkenneally.
| //! - Only perform the operation if it will not result in less than 0 data | ||
| if ((this->storedData[0].dataInstanceSum + it->baudRate * (this->currentTimestep)) >= 0){ | ||
| this->storedData[0].dataInstanceSum += it->baudRate * (this->currentTimestep); | ||
| this->storedData[0].dataInstanceSum += round(it->baudRate * (this->currentTimestep)); |
There was a problem hiding this comment.
The braces around this->currentTimestep are redundant and can be removed.
| void SimpleStorageUnit::setDataBuffer(long long int data){ | ||
| std::string partitionName = "STORED DATA"; | ||
| SimpleStorageUnit::DataStorageUnitBase::setDataBuffer(partitionName, data); | ||
| } |
There was a problem hiding this comment.
This file is missing a new line at its end. You can see that github is alerting you of this fact with the little red circle containing a dash, just to the left above this comment.
| for (int i = 0; i < partitionNames.size(); i++) | ||
| { | ||
| PartitionedStorageUnit::DataStorageUnitBase::setDataBuffer(partitionNames[i], data[i]); | ||
| } |
There was a problem hiding this comment.
New line here as well
| @param data //Vector of data to be added to each partition in partitionNames | ||
| @return void | ||
| */ | ||
| void PartitionedStorageUnit::setDataBuffer(std::vector<std::string> partitionNames, std::vector<long long int> data){ |
There was a problem hiding this comment.
This should be int64_t also, (assuming I'm not missing something here...I could be :) )
src/simulation/onboardDataHandling/storageUnit/_UnitTest/test_simpleStorageUnit.py
Show resolved
Hide resolved
Description
A method setDataBuffer was added to the dataStorageUnitBase, simpleStorageUnit, and partitionedStorageUnit modules. setDataBuffer allows the user to add or remove a specific amount of data from a given storage partition. simpleStorageUnit and partitionedStorageUnit inherit the method from dataStorageUnitBase. simpleStorageUnit only allows the user to modify the existing partition; partitionedStorageUnit allows the user to pass a vector of partition names and a vector of data to be added/removed from those partitions.
The data type for the storage capacity and storage level was changed from
doubletolong intto avoid floating-point errors during comparisons. Also, the code now checks if there is enough space for the new data before adding it to the storage unit, avoiding going over the storage capacity.Verification
Unit tests for simpleStorageUnit and partitionedStorageUnit were updated to test the new method.
Documentation
Documentation was updated to explain how the method works in each module.
Future work
N/A