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

Same Name Error handling for properties on different concepts in JointSensor #299

Open
hfaghihi15 opened this issue Oct 7, 2021 · 5 comments

Comments

@hfaghihi15
Copy link
Collaborator

For Some reason, there is a bug in handling the same property names in two separate JointSensors on two different concepts.
See the below example:

empty_entries['rows', 'cols'] = JointFunctionalReaderSensor(keyword='whole_sudoku', forward=getempties)
fixed_entries['rows', 'cols'] = JointFunctionalReaderSensor(keyword='whole_sudoku', forward=getfixed)

Printing sup in the attached function in JointSensor results in the following:

('rows', 'cols')
('rows', 'cols')-1

Now if I change the second one to

fixed_entries['rows1', 'cols']

The result would be ok as to contain:

('rows', 'cols')
('rows1', 'cols')

This seems to be a rather odd scenario maybe in the graph default property naming which we need to check on it.

@guoquan Do you have any idea where we should start for finding this error?

@guoquan
Copy link
Collaborator

guoquan commented Oct 7, 2021

I have a "namespace" set up in the construction of everything in the "graph" and make sure they won't have a name conflict.
If a node is assigned a name that is existing in the same namespace, it will get <name>-1, <name>-2, etc.
Graph class is a namespace, Concept class is a namespace, Sensor class is a namespace, same for Property class.
This ensure we can get the unambigiuos object when we do Graph.get("global") or Property.get("('rows', 'cols')-1"), from the coding perspective.
However, it is true that this is not quite human-friendly in the end.

The implementation is in regr/base.py / AutoNamed class / assign_suggest_name() method. You can override this method in Property to change the behavior (and change classmethod get() accordingly).

We need to decide how we want it to behave, anyway.

@hfaghihi15
Copy link
Collaborator Author

I have a "namespace" set up in the construction of everything in the "graph" and make sure they won't have a name conflict. If a node is assigned a name that is existing in the same namespace, it will get <name>-1, <name>-2, etc. Graph class is a namespace, Concept class is a namespace, Sensor class is a namespace, same for Property class. This ensure we can get the unambigiuos object when we do Graph.get("global") or Property.get("('rows', 'cols')-1"), from the coding perspective. However, it is true that this is not quite human-friendly in the end.

The implementation is in regr/base.py / AutoNamed class / assign_suggest_name() method. You can override this method in Property to change the behavior (and change classmethod get() accordingly).

We need to decide how we want it to behave, anyway.

Thank you for your answer, the problem is that when that number -1 is getting added to this tuple then the JointSensor doesn't work properly on separating them. Also, this may not be necessary to make properties unique unless they are defined on the same concept.

@guoquan
Copy link
Collaborator

guoquan commented Oct 23, 2021

Yea I see they don't need to be unique when they are attached to different concepts. However, I did not have a good implementation of the namespace that supports such a scenario. So I just make everything unique.

What is the issue for such naming working with JointSensor? I was expecting all the internal processes to be agnostic to the naming format of the sensors, properties, or concepts.

@hfaghihi15
Copy link
Collaborator Author

Yea I see they don't need to be unique when they are attached to different concepts. However, I did not have a good implementation of the namespace that supports such a scenario. So I just make everything unique.

What is the issue for such naming working with JointSensor? I was expecting all the internal processes to be agnostic to the naming format of the sensors, properties, or concepts.

Hi Quan,

The issue is that because the tuple becomes a string and the sensor is not able to break it anymore.

@hfaghihi15
Copy link
Collaborator Author

The same name error also happens to be problematic when defining multiple relationships and using the candidate generator sensor. because we have named keywords that should match the name of the kargs passed to the function.

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

No branches or pull requests

2 participants