Skip to content

Add TPCC QSR.#199

Merged
gatsoulis merged 10 commits into
strands-project:masterfrom
cburbridge:master
Oct 2, 2015
Merged

Add TPCC QSR.#199
gatsoulis merged 10 commits into
strands-project:masterfrom
cburbridge:master

Conversation

@cburbridge

Copy link
Copy Markdown
Member

This adds Ternary Point Calculus relations to the library. This required the addition of a triadic abstract class.
See http://www.sfbtr8.spatial-cognition.de/project/r3/QualitativeCalculi/TPCC/index.html for details on TPCC.

@gatsoulis

Copy link
Copy Markdown
Contributor

Nice. Thanks. Will test it when I get a chance. Had a look on the documentation, which reference shall I use in the documentation (there are 4-5 papers in their references list).

Only comment/suggestion I have is that I would have defined the functions in qsr_lib/src/qsrlib_qsrs/qsr_tpcc.py as class @staticmethod with a protected namespace, but your call if you want them exposed publicly and also they will appear I think in the docs too even without a docstring.

Comment thread qsr_lib/scripts/tpcc_test.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would prefer not having a separate tester for (each) qsr. All you need is add an elif statement including lines 34-40. I can fix that when I tested and make a PR to your branch.

@cburbridge

Copy link
Copy Markdown
Member Author

Yes, it was messy :-) I have tidied it up.

@gatsoulis

Copy link
Copy Markdown
Contributor

Nice thanks. I will test it when I get a chance.

@gatsoulis

Copy link
Copy Markdown
Contributor

Had a quick test. What should the output of the following data be?

o1 = [Object_State(name="o1", timestamp=0, x=0., y=0., xsize=5., ysize=8.)]
o2 = [Object_State(name="o2", timestamp=0, x=5., y=0., xsize=5., ysize=8.)]
o3 = [Object_State(name="o3", timestamp=0, x=5., y=0., xsize=5., ysize=8.)]

for o1, o2, o3?

I'm getting cbl. Is this right?

Let me make a pull request to your branch firstly.

@gatsoulis

Copy link
Copy Markdown
Contributor

Is object_distance supposed to be from the relatum or from the origin? See this line.

@gatsoulis

Copy link
Copy Markdown
Contributor

Scrap previous comment. It should be from the relatum. But I think your if-statement seems not right. Shouldn't it be:

if object_distance == 0:

(can also use q-factor in this case, i.e. if object_distance <= q-factor.

@cburbridge

Copy link
Copy Markdown
Member Author

Yes, but I made a mistake in the comparison after that. Should be 'sam'.

@gatsoulis

Copy link
Copy Markdown
Contributor

lol. just as I was writing it :P

@gatsoulis

Copy link
Copy Markdown
Contributor

What about the following:

        o1 = [Object_State(name="o1", timestamp=0, x=0., y=0., xsize=5., ysize=8.)]
        o2 = [Object_State(name="o2", timestamp=0, x=5., y=0., xsize=5., ysize=8.)]

it gives me always cbl for any of the three following values of o3.

o3 = [Object_State(name="o3", timestamp=0, x=6., y=0., xsize=5., ysize=8.)]
o3 = [Object_State(name="o3", timestamp=0, x=5., y=1., xsize=5., ysize=8.)]
o3 = [Object_State(name="o3", timestamp=0, x=6., y=1., xsize=5., ysize=8.)]
o3 = [Object_State(name="o3", timestamp=0, x=4., y=0., xsize=5., ysize=8.)]
o3 = [Object_State(name="o3", timestamp=0, x=4., y=-1., xsize=5., ysize=8.)]

But got a dbl for

o3 = [Object_State(name="o3", timestamp=0, x=50., y=50., xsize=5., ysize=8.)]

@cburbridge

Copy link
Copy Markdown
Member Author

I made a mess when "doing a quick tidy up" to the code earlier, latest commit should fix it.

@gatsoulis

Copy link
Copy Markdown
Contributor

For o1, o2 always the same and as follows:

o1 = [Object_State(name="o1", timestamp=0, x=10., y=10., xsize=5., ysize=8.)]
o2 = [Object_State(name="o2", timestamp=0, x=15., y=10., xsize=5., ysize=8.)]

For o3:

cfr

o3 = [Object_State(name="o3", timestamp=0, x=14., y=10., xsize=5., ysize=8.)]

cbl

o3 = [Object_State(name="o3", timestamp=0, x=16., y=10., xsize=5., ysize=8.)]

clf

o3 = [Object_State(name="o3", timestamp=0, x=15., y=11., xsize=5., ysize=8.)]

crb

o3 = [Object_State(name="o3", timestamp=0, x=15., y=9., xsize=5., ysize=8.)]

Does these sound right? And when am I supposed to get cs* relations?

If they are not correct, maybe try to check the values before re-pushing? :P

@cburbridge

Copy link
Copy Markdown
Member Author

This is right, ignoring the "s" relations which I don't bother with, see TODO on line https://github.com/cburbridge/strands_qsr_lib/blob/master/qsr_lib/src/qsrlib_qsrs/qsr_tpcc.py#L32. These "same" relations are very unlikely to occur in real floating point data...but ok I'll add them before this can be merged!

@gatsoulis

Copy link
Copy Markdown
Contributor

I see. Thanks. That's why I got confused. Happy to merge as it is and make an issue about it. Up to you whether you want to deal with it now or later.

@gatsoulis

Copy link
Copy Markdown
Contributor

Putting it in progress, let me know what you want to do per message ^^.

@gatsoulis gatsoulis assigned cburbridge and unassigned gatsoulis Oct 1, 2015
@cburbridge

Copy link
Copy Markdown
Member Author

I have added the s relation. It won't ever come up in real data so it might be nicer to have some sort wider band for that relation, but it's not important now. Please merge this and if there are any further problems people can open issues for me to fix.

As a side note, I had a quick look at your unittests, but could not quickly see how I'm supposed to add one in your style. Can I just make a text file with the input and another with the expected QSRs?

@gatsoulis

Copy link
Copy Markdown
Contributor

Thanks!

Please rebase/resolve conflicts (would be faster than me doing it and making a PR to your branch), have a feeling it's only the qsrs/init.py.

Don't worry about the unnitest, I can easily do it as I need to update the multiple one also. Waiting for this PR to be merged before doing so though.

@cburbridge

Copy link
Copy Markdown
Member Author

done.

@gatsoulis

Copy link
Copy Markdown
Contributor

I have added the s relation. It won't ever come up in real data so it might be nicer to have some sort wider band for that relation, but it's not important now. Please merge this and if there are any further problems people can open issues for me to fix.

You can use a threshold "q_factor" for that, used in others. Name it as quantisation_factor when you do and blame @cdondrup for using british spelling in programming :P

gatsoulis pushed a commit that referenced this pull request Oct 2, 2015
@gatsoulis
gatsoulis merged commit ef610fd into strands-project:master Oct 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants