Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lb/new clean #65

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Lb/new clean #65

wants to merge 10 commits into from

Conversation

Kriplingo
Copy link
Collaborator

This is the new "minimal" cleaned version of the code.
Here is the list of changes:

  1. Removed all outdated files. (This includes jupyter notebooks, unused python code, images for the use in notebooks, unused encodings etc.)
  2. Divided the racks example constraints into two different files. One has the attribute-oriented constraints, the other the object-oriented constraints. (The attribute ones do not seem to significantly affect performance so they are included in the encodings.)
  3. Renamed ooasp_simple.lp to ooasp.lp since this is the new current main encoding.
  4. Removed redundant dependencies and updated requirements.txt accordingly
  5. Updated the README.md to include information about installing using the requirements.txt for the users who would like to avoid using poetry.
  6. Added type hints where it seemed appropriate.
  7. Removed a redundant (most likely forgotten) print statement, which was ruining the app.py output formatting.
  8. Changed the #show statements as proposed in teams in order to make the output easier to work with and reusable.

@Kriplingo
Copy link
Collaborator Author

Please upon review let me know how to handle theDOC.md file, as it too is outdated, but in my opinion still contains some interesting details and descriptions (e.g. multi-shot grounding) and explanations of the tasks etc.

rtaupe
rtaupe previously approved these changes Oct 16, 2024
Copy link
Collaborator

@rtaupe rtaupe left a comment

Choose a reason for hiding this comment

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

To me everything looks fine, thank you!
Let's wait with the merge until we also have a review by @susuhahnml.

@rtaupe
Copy link
Collaborator

rtaupe commented Oct 16, 2024

Please upon review let me know how to handle theDOC.md file, as it too is outdated, but in my opinion still contains some interesting details and descriptions (e.g. multi-shot grounding) and explanations of the tasks etc.

I propose to keep all the information in the file, but move the outdated parts to a new section in the bottom, in which we also state that these features are only available in an old version.
@Kriplingo do you think you can do this?

@Kriplingo
Copy link
Collaborator Author

Please upon review let me know how to handle theDOC.md file, as it too is outdated, but in my opinion still contains some interesting details and descriptions (e.g. multi-shot grounding) and explanations of the tasks etc.

I propose to keep all the information in the file, but move the outdated parts to a new section in the bottom, in which we also state that these features are only available in an old version. @Kriplingo do you think you can do this?

Certainly! Should I make the proposed changes and send you a version for review directly, or would it be better to keep the review process for this file on github?

@rtaupe
Copy link
Collaborator

rtaupe commented Oct 16, 2024

Please upon review let me know how to handle theDOC.md file, as it too is outdated, but in my opinion still contains some interesting details and descriptions (e.g. multi-shot grounding) and explanations of the tasks etc.

I propose to keep all the information in the file, but move the outdated parts to a new section in the bottom, in which we also state that these features are only available in an old version. @Kriplingo do you think you can do this?

Certainly! Should I make the proposed changes and send you a version for review directly, or would it be better to keep the review process for this file on github?

Let's do it here on GitHub

nrueh
nrueh previously approved these changes Oct 21, 2024
@nrueh
Copy link
Collaborator

nrueh commented Oct 21, 2024

Sorry for the delay, Susana and I were on LPNMR and ICLP last week. The changes look good, thank you!
I suppose @susuhahnml will also do her review soon :)

@Kriplingo
Copy link
Collaborator Author

I have reorganised the DOC.md file so that irrelevant things for this version are no-longer included, plase someone check if everything there is correct and still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fonts for visualisation require German version of fonts.
4 participants