RangeAbstractions now abstract to Maps from identifiers to values instead of…
Auditedcb3a24c0eeae
Actions

Referenced Files

None

Subscribers

goetzinger

• juhasz

Description

RangeAbstractions now abstract to Maps from identifiers to values instead of Vectors ov values only.

Default results of partial functions can now be hidden (since order is
not important any more) by passing true as a second argument to the
RangeAbstraction constructor.

Details

Auditors

• juhasz

Provenance

• btutzer	Authored on May 2 2019, 2:31 PM
• btutzer	Pushed on May 2 2019, 2:33 PM

Parents

R20:bf9e6af9b956: Renamed LinearFunctions to FunctionAbstractions and extracted RangeConfidence…

Branches

Unknown

Tags

Unknown

Event Timeline

• btutzer committed R20:cb3a24c0eeae: RangeAbstractions now abstract to Maps from identifiers to values instead of… (authored by • btutzer).May 2 2019, 2:31 PM

@goetzinger RangeConfidence now returns Maps as agreed upon in todays meeting.

• btutzer added a subscriber: • juhasz.May 2 2019, 2:40 PM

• btutzer added inline comments.

/include/rosa/agent/Abstraction.hpp
36	@juhasz I now implemented an additional feature to make this as similar to maxi's original implementation as possible. When a PartialFunction is evaluated to it's default value in the functor of RangeConfidence, it can be ignored. This is because with maps we don't need the values to be in order any more. To achieve this, I had to make the Default value public in the Abstraction class. Are you ok with that? Otherwise I'll revert back and delete that feature.

• juhasz raised a concern with this commit.May 2 2019, 3:16 PM

• juhasz added inline comments.

/include/rosa/agent/Abstraction.hpp
36	I would be OK with making the field public, as it is constant, if it would be for good reason...
/include/rosa/agent/RangeConfidence.hpp
87	An Abstraction returning a value that equals to its Default value does not necessary mean that Default itself was returned (i.e., the Abstraction was "undefined" in the evaluated point); but it could mean that the actual defined value of the Abstraction happened to coincide with Default. For example, the following partial function evaluates to 0 at 0, which also happen to be is its default value: PartialFunction<float, float>({ {{-1, 1}, std::make_shared<LinearFunction<float, float>>(0, 1)} }, 0) I would leave Abstraction::Default as it was and rather add a new member function to Abstraction: bool isDefaultAt(const T&)const noexcept; telling whether the Abstraction falls back to Default at a given position. I think that would express the original intention better -- if I understood correctly.

This commit now has outstanding concerns.May 2 2019, 3:16 PM

Addressed in 783f8c48d55e

This commit now requires verification by auditors.May 2 2019, 4:37 PM

783f8c48d55e looks good!
Could you please complete the documentation comments for the new function before I accept this commit?

• juhasz added inline comments.May 3 2019, 9:52 AM

/include/rosa/agent/RangeConfidence.hpp
63	One more detail came to my mind: the categories are supposed to be unique in Abstractions. Is that correct? If that is correct, that should be a precondition of the constructor, which should be documented and validated in an `ASSERT` in the constructor to ensure that the class is used in a correct way.

• btutzer added inline comments.May 3 2019, 12:23 PM

/include/rosa/agent/RangeConfidence.hpp
63	Since the categories are keys to a map, they are inherently unique, aren't they @juhasz?

On another note: I tried to stick to your coding-style as much as possible. This included your naming convention of template typenames (T, A, ...). As RangeConfidence requires three such typenames, this get's a little out of hand. I'd rather call them DomainType (T), IdType (A) and RangeType (B), would you be ok with that @juhasz?

In R20:cb3a24c0eeae370bda4839f7748c8ce7933576e9#8648, @btutzer wrote:

On another note: I tried to stick to your coding-style as much as possible. This included your naming convention of template typenames (T, A, ...). As RangeConfidence requires three such typenames, this get's a little out of hand. I'd rather call them DomainType (T), IdType (A) and RangeType (B), would you be ok with that @juhasz?

Absolutely!

As for the style I followed in the deluxe interface, I tried to use letters or letter combinations for type arguments so that they somehow relate to the role of the type (e.g., input type is T and master-input type is MT). In this case, it would be similar to use D for domain type, I for id type, and R for range type.
Nevertheless, your suggestion with writing out words is also completely fine.

/include/rosa/agent/RangeConfidence.hpp
63	Correct. My bad, sorry.

In R20:cb3a24c0eeae370bda4839f7748c8ce7933576e9#8619, @juhasz wrote:

783f8c48d55e looks good!
Could you please complete the documentation comments for the new function before I accept this commit?

Addressed in 040533b7e00b

In R20:cb3a24c0eeae370bda4839f7748c8ce7933576e9#8650, @juhasz wrote:

In R20:cb3a24c0eeae370bda4839f7748c8ce7933576e9#8648, @btutzer wrote:

On another note: I tried to stick to your coding-style as much as possible. This included your naming convention of template typenames (T, A, ...). As RangeConfidence requires three such typenames, this get's a little out of hand. I'd rather call them DomainType (T), IdType (A) and RangeType (B), would you be ok with that @juhasz?

Absolutely!

As for the style I followed in the deluxe interface, I tried to use letters or letter combinations for type arguments so that they somehow relate to the role of the type (e.g., input type is T and master-input type is MT). In this case, it would be similar to use D for domain type, I for id type, and R for range type.
Nevertheless, your suggestion with writing out words is also completely fine.