GEGlobalResearch / DARPA-ASKE-TA1

ANSWER Project to demonstrate knowledge-driven extraction of scientific models from code and texts
Other
7 stars 5 forks source link

Units not properly handled in expressions #55

Open crapo opened 4 years ago

crapo commented 4 years ago

@kittaakos , SadlASTUtils needs to be more robust to handle units in expressions, which shows up when doing Dialog model processing on the examples used yesterday for Suitability grammar. Can we have a tcon at your convenience to discuss?

kittaakos commented 4 years ago

Can you please describe the expected behavior with a few examples? I can convert them to actual tests later. As I see SadlASTUtils is a static utility class so I am not sure we can customize it from the Dialog and rebind the injection. We might have to change it in SADL instead.

crapo commented 4 years ago

The current implementation of isUnit uses classes from the grammar (QueryStatement, EquationStatement) to determine if something is a unit construct. This can't be extended because it cannot depend on (import) the Dialog grammar classes. If it actually needs to know what is the containing grammar class, which I don't think it does, then it would need to make a call to the relevant model processor to determine that. I have solved the problem in the Dialog model processor, I think, so for the moment we can postpone consideration of this issue. Let's come back to it only if needed. I'm leaving it open for the moment until we have greater insight.

kittaakos commented 4 years ago

Thanks for the update!

I have solved the problem in the Dialog model processor, I think

Great!

I'm leaving it open for the moment Good idea. We would have to transform the AST utils from a static utility class to a normal injectable class, subclass it for dialog and rebind it.

Please update this issue if we have to change the AST utils the way 👆 I recommended.