[iss-45] OrdSetsAndMaps by LucasCavagna · Pull Request #53 · CIFASIS/sb-graph

LucasCavagna · 2025-03-26T19:19:16Z

I decided to implement a new version for all ordered set operations. I used linked lists and different conditions to minimize the number of operations in the intersection process, supporting sets with multiple dimensions and variable steps. Then, I wrote a new version for union, difference, minus, and complement operations to also support multiple dimensions and variable steps.

LucasCavagna · 2025-04-10T21:37:55Z

Commit from Apr 10, 2025

MODIFICATIONS:

Ordered intersection has been modified — not optimal yet.

interForComl has been added — not the final version.

Ordered complement has been modified — still not final.

Multi-dimensional tests have been added to set_perf.

COMMENTS: The changes to the ordered complement have resulted in improved performance during the tests — approximately 5X faster than the previous implementation.

LucasCavagna · 2025-04-16T20:34:09Z

Commit from Apr 16, 2025

MODIFICATIONS:

The ordered intersection has been updated — this is supposed to be the final version.

interForComl has been modified — we removed unnecessary intersections.

The ordered complement has been updated — almost final version. In the end, there's no need to use the Boost library or perform sorting, either here or in the intersection.

Multi-dimensional tests have been added to set_perf — specifically those that test the conditions added to interForComl.

COMMENTS:
The changes to the ordered complement have led to improved performance during testing — approximately 2X faster than the previous implementation.

LucasCavagna · 2025-04-23T19:35:36Z

Commit from Apr 23, 2025

MODIFICATIONS:

All operations for ordered pwmaps have been implemented except for: reduce, minMap, minAdj, and compact — these are not final versions yet.

The emplaceHint function was added. It inserts a map element into an ordered pwmap in an ordered manner based on a given hint — it attempts to place the element linearly starting from the hint.

COMMENTS:
Operations on ordered pwmaps generally perform as well as or better than those on unordered pwmaps. However, it is only due to the image of the maps that certain operations—like inverse—end up being more costly.

LucasCavagna · 2025-05-12T18:27:12Z

Commit from May 12, 2025

MODIFICATIONS:

Minimal updates applied to orderedSet operations.

Special cases added to enhance behavior in certain operations.

Refactored interForCompl and intersection to separate the search criterion from the search starting point selection.

Variable names updated to be more representative and improve code clarity.

COMMENTS:
These changes are minor and do not significantly impact the performance of operations on orderedSets.

LucasCavagna · 2025-06-08T15:09:52Z

Commit from Jun 8, 2025

MODIFICATIONS:

A bug in the synthetic test cases that caused a segmentation fault was fixed.

COMMENTS:

The current implementation of minimum and maximum perimeter recalculates each time they are needed; it would be good to avoid this without negatively affecting the maps implementation when they are unordered.

Kalashnikovni · 2025-06-09T15:36:07Z

I suggest giving English names to all variables and functions to keep the same style for all the project.

Kalashnikovni · 2025-06-11T16:30:23Z

+      auto liPrev = indices.before_begin();
+      auto liCurr = indices.begin();
+
+      bool doInt = (!(elementMax.menorThan(mdi.minElem())) && !(mdi.maxElem().menorThan(elementMin)));


In line 1163 Is the left side of the && necessary? We check that condition while advancing pos in line 1140.

It was resolved with doInt auxiliary function, but

Kalashnikovni · 2025-06-11T16:38:03Z

+
+
 Map::~Map() {}
-Map::Map(const SetAF &fact) : fact_(fact), dom_(fact.createSet()) {}


Keep the writing style for all Map constructors.

LucasCavagna · 2025-06-12T19:36:00Z

Commit from Jun 12, 2025

MODIFICATIONS:

All changes committed are based on the previous comments.

Some auxiliary functions have been added to set.cpp for OrderedSets.

COMMENTS:

There is a conflict with some files. I suppose this is because Git is comparing the branches in preparation for a possible merge.

Kalashnikovni · 2025-06-18T14:53:00Z

Changes in pw_map.* are missing because we are getting errors while building saying that menorThan is non-existent now.

Kalashnikovni · 2025-07-09T19:41:14Z

In general we have used camel case for function names, and for variables underscores:

void someFunction() {
  int my_variable = 10;
  return x*my_variable;
}

Kalashnikovni · 2025-06-18T12:52:21Z

-  if (i.isEmpty())
-    intervals_ = InterVector();
+{ 
+  if (i.isEmpty()){


It's silly, but braces are not necessary.

Kalashnikovni · 2025-07-07T22:42:29Z


-bool PWMap::operator==(const PWMap &other) const
+
+bool mapEntryComp(const MapEntry &mpe1, const MapEntry &mpe2) 


Maybe we can define bool operator<(const MapEntry &mpe1, const MapEntry &mpe2) instead of mapEntryComp?

Kalashnikovni · 2025-07-08T13:08:25Z

+{ 
+  if (!m.dom().isEmpty()){
+    MapEntry mpe = createMapEntry(m);
+    if (pieces_.empty() || pieces_.back().second.first < mpe.second.first) 


Use mapEntryComp instead of pieces_.back().second.first < mpe.second.first.

Kalashnikovni · 2025-07-08T21:31:16Z

+    Map resComb = fact_.createMap(om.dom(), om.exp());
+
+    if (doInt(osp, tsp)){
+      Set oDom = om.dom(), new_dom = oDom.difference(dom());


dom() can be calculated out of the loop.

Kalashnikovni · 2025-07-09T19:35:53Z

 {
-  return delegate_->filterMap(f);
+  OrdPWMap res(fact_);
+  if (subdom.isEmpty())


As in firstInv: if (isEmpty() || subdom.isEmpty()) can be used.

Kalashnikovni · 2025-07-09T19:53:01Z

+  Interval all(0, 1, Inf);
+  Set univ = fact_.createSet(SetPiece(arity(), all));
+  OrdPWMapCRef othr = static_cast<OrdPWMapCRef>(other);
+  processMapsOrd(othr,univ, static_cast<OrdPWMapRef>(*res) , &OrdPWMap::processMinus,true);


If the purpose of the Set argument of processMapsOrd is to return a value (for example, like in equalImage), univ here should not be passed as argument.
Then, in minAdjMap is used to keep a register for visited values in the domain. I think that is better to modify the function processMapsOrd to take two Sets as arguments: one for input and the other for output.

Kalashnikovni · 2025-07-09T19:55:44Z

+unsigned int OrdPWMap::emplaceHint(const Map &m ,unsigned int hint)
 {
-  return delegate_->operator+(*other.delegate_);
+  auto end = pieces_.end();


Check if m.dom().isEmpty() before inserting the element in the pw. This way in other functions:

if (!m.dom().isEmpty()) emplaceHint(...);

can be directly substituted by:

emplaceHint(...);

Kalashnikovni · 2025-07-09T19:58:04Z

+  for (const MapEntry &mpe : pieces_){
+    const Map &inv = mpe.first.minInv();
+    if(!inv.dom().isEmpty())
+      res.pieces_.emplace_back(createMapEntry(inv));


Use OrdPWMap::emplaceBack.

[iss-45]

3ec5c11

LucasCavagna added the enhancement New feature or request label Mar 26, 2025

LucasCavagna added this to the SB-Graph 4.0.0 milestone Mar 26, 2025

LucasCavagna requested a review from Kalashnikovni March 26, 2025 19:19

LucasCavagna self-assigned this Mar 26, 2025

LucasCavagna linked an issue Mar 26, 2025 that may be closed by this pull request

OrdMultiDimSets #45

Closed

Kalashnikovni added this to SB-Graph library issue tracker. Mar 26, 2025

Kalashnikovni moved this to To do in SB-Graph library issue tracker. Mar 26, 2025

Kalashnikovni moved this from To do to In progress in SB-Graph library issue tracker. Mar 26, 2025

[iss-45]

4ede09b

Kalashnikovni requested review from LucioTrincheri, francosansone and joaquinffernandez April 1, 2025 18:32

LucasCavagna added 2 commits April 3, 2025 17:46

[iss-45]

af5ce43

[iss-45]

54db42f

[iss-45]

00131b3

Kalashnikovni modified the milestones: SB-Graph 4.0.0, SB-Graph 5.0.0 Apr 22, 2025

[iss-45]

7adf652

LucasCavagna added 2 commits April 30, 2025 17:11

[iss-45]

7a2e370

[iss-45]

d93ca0c

Kalashnikovni and others added 5 commits May 21, 2025 14:40

Update c-cpp.yml

51b2f27

Update Makefile.in

8bd445b

Update c-cpp.yml

d62f854

Update Makefile.in

6993fcd

[iss-45]

1833fa7

[iss-45]

294d963

Kalashnikovni reviewed Jun 9, 2025

View reviewed changes

Comment thread sbg/natural.cpp

Kalashnikovni reviewed Jun 9, 2025

View reviewed changes

Comment thread sbg/set.hpp

Kalashnikovni removed request for LucioTrincheri, francosansone and joaquinffernandez June 9, 2025 15:41

Kalashnikovni reviewed Jun 11, 2025

View reviewed changes

[iss-45]

9cdf271

Kalashnikovni mentioned this pull request Jun 18, 2025

Fixed points of PWMap #77

Closed

[iss-45]

f0f0c62

LucasCavagna and others added 10 commits July 16, 2025 14:37

[iss-45]

add17f6

[iss-45]

7d19f3d

[iss-45]

c6ad869

[iss-45]

013849c

[iss-45]

c61a5a8

[iss-45]

341c57b

[iss-45]

ae7c9c9

[iss-45]

b676291

[iss-45]

69fdd84

[iss-45]

53d2022

Kalashnikovni approved these changes Sep 11, 2025

View reviewed changes

Kalashnikovni added 2 commits November 3, 2025 20:39

Merge sb-graph-dev into iss-45-ordmultidimsets

3596870

Workflow correction

1d70c13

Kalashnikovni merged commit 0b9a099 into sb-graph-dev Nov 3, 2025
2 checks passed

Kalashnikovni moved this from In progress to Done in SB-Graph library issue tracker. Jan 14, 2026



		Map::~Map() {}
		Map::Map(const SetAF &fact) : fact_(fact), dom_(fact.createSet()) {}


		bool PWMap::operator==(const PWMap &other) const

		bool mapEntryComp(const MapEntry &mpe1, const MapEntry &mpe2)

Uh oh!

Conversation

LucasCavagna commented Mar 26, 2025

Uh oh!

LucasCavagna commented Apr 10, 2025

Uh oh!

LucasCavagna commented Apr 16, 2025

Uh oh!

LucasCavagna commented Apr 23, 2025

Uh oh!

LucasCavagna commented May 12, 2025

Uh oh!

LucasCavagna commented Jun 8, 2025

Uh oh!

Uh oh!

Uh oh!

Kalashnikovni commented Jun 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LucasCavagna commented Jun 12, 2025

Uh oh!

Kalashnikovni commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kalashnikovni commented Jul 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kalashnikovni commented Jun 18, 2025 •

edited

Loading