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

Large scale cel #495

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

Large scale cel #495

wants to merge 19 commits into from

Conversation

Jean-KOUAGOU
Copy link
Collaborator

@Jean-KOUAGOU Jean-KOUAGOU commented Nov 22, 2024

  • Added python scripts to run TDL and Drill on DBpedia learning problems with "https://dbpedia.data.dice-research.org/sparql" as knowledge source.

  • Updated README with Bash commands to start class expression learning

  • Also fixed some typos in README, e.g., Learining OWL Class Expressions instead of Learining OWL Class Expression

  • I can add the Bash command for DL-Learner as well before merge, but execution freezes after some minutes. It is unable to solve even a single learning problem

@github-actions github-actions bot temporarily deployed to pull request November 22, 2024 09:22 Inactive
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that we need have two different script that only differs by selected learner.
Perhaps, you can use argparse to select learner

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think that we need have two different script that only differs by selected learner. Perhaps, you can use argparse to select learner

I merged both files into dbpedia_concept_learning_with_ontolearn.py

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that there is a need to merge a script into devl branch that doesn't work.
So, either ensure that this code is compartiable with the dev branch or remove it from your PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think that there is a need to merge a script into devl branch that doesn't work. So, either ensure that this code is compartiable with the dev branch or remove it from your PR.

Removed this file

Copy link
Member

Choose a reason for hiding this comment

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

Your changes in the init decrease the quality of code by

  1. adding more lines than required
  2. multiple assertions without unique information

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Your changes in the init decrease the quality of code by

  1. adding more lines than required
  2. multiple assertions without unique information

Resolved, see below

@@ -348,3 +351,316 @@ def fit_from_iterable(self, dataset: List = None, max_runtime=None) -> List[Dict
self.max_runtime = max_runtime

return [self.fit(pos=p, neg=n, max_runtime=self.max_runtime).best_hypothesis() for (s, p, n) in dataset]


class DLLearnerBinderSPARQL:
Copy link
Member

Choose a reason for hiding this comment

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

We do not need to create yet another binder.
Please ensure that the content of this class in DLLearnerBinder.

Copy link
Collaborator Author

@Jean-KOUAGOU Jean-KOUAGOU Nov 22, 2024

Choose a reason for hiding this comment

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

We do not need to create yet another binder. Please ensure that the content of this class in DLLearnerBinder.

Removed DLLearnerBinderSPARQL, added its content into DLLearnerBinder

@github-actions github-actions bot temporarily deployed to pull request November 22, 2024 14:06 Inactive
@github-actions github-actions bot temporarily deployed to pull request November 22, 2024 14:28 Inactive
README.md Outdated
@@ -118,6 +118,20 @@ print(owl_expression_to_sparql(expression=h))
save_owl_class_expressions(expressions=h,path="owl_prediction")
```

- With one command
Copy link
Member

Choose a reason for hiding this comment

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

Please remove these lines. There is no need to repeat exact same commands multiple times

Copy link
Collaborator Author

@Jean-KOUAGOU Jean-KOUAGOU Nov 22, 2024

Choose a reason for hiding this comment

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

Should I remove all three commands? From line 121 onwards.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I removed the three commands

@Jean-KOUAGOU Jean-KOUAGOU removed the request for review from alkidbaci November 22, 2024 14:41
@github-actions github-actions bot temporarily deployed to pull request November 22, 2024 14:45 Inactive
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.

2 participants