Skip to content

[iss-70] Parser+operator-+eval msg#72

Merged
Kalashnikovni merged 8 commits into
sb-graph-devfrom
iss-70-minor-enhancements
Jun 12, 2025
Merged

[iss-70] Parser+operator-+eval msg#72
Kalashnikovni merged 8 commits into
sb-graph-devfrom
iss-70-minor-enhancements

Conversation

@Kalashnikovni

@Kalashnikovni Kalashnikovni commented May 20, 2025

Copy link
Copy Markdown
Collaborator

See related issue.

@Kalashnikovni Kalashnikovni added this to the SB-Graph 5.0.0 milestone May 20, 2025
@Kalashnikovni Kalashnikovni self-assigned this May 20, 2025
@Kalashnikovni Kalashnikovni added bug Something isn't working enhancement New feature or request labels May 20, 2025
@Kalashnikovni Kalashnikovni linked an issue May 20, 2025 that may be closed by this pull request
@Kalashnikovni

Copy link
Copy Markdown
Collaborator Author
  • Parse intervals without step.
  • UnordPWMap::operator-.
  • Error messages in eval_expr.cpp.

@Kalashnikovni Kalashnikovni changed the title Parser+operator-+eval msg [iss-70] Parser+operator-+eval msg May 20, 2025
@Kalashnikovni

Copy link
Copy Markdown
Collaborator Author
  • Debugged UnordPWMap::operator-.
  • Implemented minMap for multi-dimensional case.
  • Now member_class macro declares a function that returns a const reference to the member of the class.
  • Automatic selection of optimal set implementation.

@Kalashnikovni

Copy link
Copy Markdown
Collaborator Author

@joaquinffernandez @francosansone Does it makes sense that the partitioner inside SBG compiles with the library? If not, I can modify the Makefile. I think that the algorithms should be part of the library too.
P.S. I will also move the other algorithms (matching, SCC, etc.) inside the algorithms/ directory.

@Kalashnikovni Kalashnikovni moved this from Backlog to In progress in SB-Graph library issue tracker. Jun 2, 2025
@francosansone

Copy link
Copy Markdown
Collaborator

@joaquinffernandez @francosansone Does it makes sense that the partitioner inside SBG compiles with the library?

Yes, I think it does. I think the main file should link the library in compilation time, since it should not depend on the changes we do on the implementation and it also gives us an use case to test if the library works.

If not, I can modify the Makefile. I think that the algorithms should be part of the library too. P.S. I will also move the other algorithms (matching, SCC, etc.) inside the algorithms/ directory.

I totally agree, except for the main file, the rest of the code should part of the sb-graph library. In the future we could think about if we want to split it as a new library (for example, if sb-graph becomes too large).

@joaquinffernandez

Copy link
Copy Markdown
Collaborator

If not, I can modify the Makefile. I think that the algorithms should be part of the library too. P.S. I will also move the other algorithms (matching, SCC, etc.) inside the algorithms/ directory.

I agree, in the future we might consider split the functionalities if it becomes too big, but for the moment I think we are OK

@Kalashnikovni

Copy link
Copy Markdown
Collaborator Author
  • Updated Makefiles.

@Kalashnikovni

Copy link
Copy Markdown
Collaborator Author
  • All SBG algorithms are now in the algorithms/ directory.

@Kalashnikovni Kalashnikovni moved this from In progress to Review in SB-Graph library issue tracker. Jun 9, 2025
@Kalashnikovni Kalashnikovni marked this pull request as ready for review June 9, 2025 14:55
@Kalashnikovni

Copy link
Copy Markdown
Collaborator Author

@joaquinffernandez @francosansone This is ready to be merged into sb-graph-dev. I'll wait for your answers.

@francosansone

Copy link
Copy Markdown
Collaborator

readme.md file should be updated. Step 3 in basic installation should be changed to something like:

  1. Type make compile to compile the library libsbgraph.a and some executable files.

@francosansone

Copy link
Copy Markdown
Collaborator

Not related to this PR, but commands make install and make install-folders should add header files in algorithms/partitioner/. Currently, functions and classes defined there are not accessible from sb-graph library users.

@joaquinffernandez joaquinffernandez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 👏

}

map_exp.offset_ref() = offset; // this works like `map_exp.set_offset(offset);`
LExp map_exp(var_exp.slope(), offset);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@francosansone francosansone left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@joaquinffernandez

Copy link
Copy Markdown
Collaborator

Regarding the Makefile changes, the overall structure is quite better now, I will change the build system to cmake but now it's much easier to migrate.
When that's done I can modify the include rules and readme files.

@Kalashnikovni

Copy link
Copy Markdown
Collaborator Author

Thanks for all the help =)

@Kalashnikovni Kalashnikovni reopened this Jun 12, 2025
@Kalashnikovni Kalashnikovni merged commit 14d3c3c into sb-graph-dev Jun 12, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

Development

Successfully merging this pull request may close these issues.

Minor enhancements

3 participants