sudeep87 / uimafit

Automatically exported from code.google.com/p/uimafit
0 stars 0 forks source link

checking for mandatory values of configuration parameters when creating descriptors #5

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The following conversation summarizes the issue:

Philip:
I think it should be possible to create an AED with mandatory
configuration parameters missing their values.  I am not sure what the use
case is -
but it doesn't seem that UIMA cares if the AED is complete or not until the
moment
that you actually instantiate the AE.  I have extended a unit test called
org.uutuc.factory.AnalysisEngineFactoryTest.testReflectPrimitiveDescription()
which
demonstrates what I think is acceptable behavior.  It creates an AED with
the values
for the mandatory parameters PARAM_FLOAT_3 and PARAM_FLOAT_6 missing.  At
the bottom
of the unit test I demonstrate that the AE can still be instantiated so long as
parameter values are passed in with the createPrimitive method.  

Richard:
The way mandatory parameters are checked comes from the fact that my
initial approach
was to interpret the annotations after the component had already been
instantiated. I
suppose
I should check for the presence of mandatory parameters, only when the
ComponentConfigurator
is running in "apply" mode (with an actual component instance provided) and
ignore
them when
running in "configure" mode.

On the other side: when editing a component in the Eclipse Descriptor
Editor and
saving it with
mandatory parameters not filled, it will display an error message. A
descriptor with
mandatory
parameters not set is - in my opinion - faulty as it is incomplete. When a
PEAR is
installed it will
also try to run the component based on the descriptor. If the descriptor is
incomplete, again it
will produce an error.

My use-case for incomplete descriptors would be to use them as "template
descriptors", that is to
save people from writing huge descriptors and they just have to fill in the
missing
values. But this
is exactly the case we had here in Darmstadt and which we are getting rid
of now
using UUTUC
(DDF? ;)) to dynamically create descriptors when we need them.

I would personally opt for always treating missing mandatory values as an
error until
somebody comes
up with a good use-case.

Philip:
ok - I am cool with that.  I don't have a meaningful use-case - only a unit
test that
relies on it (which can be changed!)  If nothing else, this pointed out the
need to
me to extend the createPrimitive(AED) method. 

Philip:
Oops!  I thought of a very important use case for why you do not want to
require that
mandatory parameters have values - when you are generating the XML
descriptor files.
 In ClearTK we are moving towards having all of the XML descriptor files
that we make
available be generated.  The way we do this is by using UUTUC to create an
AED and
then write it to XML using the toXML method.  Here it is quite common to have a
configuration parameter that is required, but no value is provided, at the
time the
AED is instantiated. 

Richard:
That's what I meant with "template descriptors". But what do you use the
incomplete descriptors for?

Philip:
sorry, I didn't catch your meaning before.  

well, we don't actually use them ourselves.  but when you release a UIMA
library
without any descriptor files for your components that introduces a barrier
to folks
who have never used UUTUC and just want to run ClearTK components the
good-old-fashioned way - using descriptor files.  So, we aim to provide a
complete
set of descriptor files for the convenience of users.  

Richard:
Even though I don't feel very happy with it, I see that you have a valid
point here. The best compromise I can 
currently come up with would be static variable or system property
controlling if checking for mandatory values 
takes place or not. Thus you could turn the checking off while generating
the descriptors.

Philip:
I don't like the idea of using a static variable or system property here. 
Here are
two other possibilities:

1) add a boolean parameter to e.g. createPrimitive that determines whether
to check
for values of mandatory configuration parameters.  If there is no value for a
mandatory configuration parameter, then an exception will be thrown.

advantages - allows you to call createPrimitiveDescription with a single
method call
disadvantages - I think its a little awkward to pass in a boolean which
determines
whether or not you want an exception to be thrown.  It also clutters the
createPrimitiveDescription method which already has 6 parameters and will
likely have
more as the class becomes more complete.  It also requires that we do the
same for
CollectionReaderFactory.  

2) add a new method (such as "validateMandatoryValues") which takes a
descriptor and
checks to see if all the mandatory values are satisfied.  
advantages - presumably we only need a single method to handle both AEDs
and CRDs. 
It will make a cleaner api. 
disadvantages - now you will need to make two method calls where before you
just
needed one.  

I vote for 2.

Steve:
I also vote for 2. It's more explicit, and it will be more obvious in the
traceback
why an exception is being thrown.

Richard:
As long as InitializeUtil checks for the presence of parameters annotated
as being mandatory even if the 
descriptor says that they are not mandatory, I am fine with not having
extra checks when creating the 
descriptors. Cf. comment in InitializeUtil:99.

Philip:
Makes sense to me.  I take it that InitializeUtil already does the
necessary checks -
or will make them when you commit your recent patch to trunk?  If so, then
I am fine
with closing this issue and not implementing the above proposed
"validateMandatoryValues" method above.  

Steve:
I also think it makes sense that InitializeUtil is the right place to be
checking for
mandatory values.

Richard:
One problem with the message thrown by UIMA about missing parameters is,
that it doesn't say for which 
component they are missing... and that is thrown before InitializeUtil ever
can check for mandatory values:

Caused by: org.apache.uima.resource.ResourceConfigurationException: No
value has been assigned to the 
mandatory configuration parameter InputPath.
    at 
org.apache.uima.resource.impl.ConfigurationManagerImplBase.validateConfiguration
ParameterSettings(Config
urationManagerImplBase.java:539)
    at 
org.apache.uima.resource.impl.ConfigurationManagerImplBase.validateConfiguration
ParameterSettings(Config
urationManagerImplBase.java:491)
    at 
org.apache.uima.resource.impl.ConfigurationManagerImplBase.createContext(Configu
rationManagerImplBase.
java:145)
    at
org.apache.uima.resource.Resource_ImplBase.initialize(Resource_ImplBase.java:128
)

Original issue reported on code.google.com by pvogren@gmail.com on 1 May 2010 at 2:57

GoogleCodeExporter commented 8 years ago

Original comment by pvogren@gmail.com on 1 May 2010 at 5:35

GoogleCodeExporter commented 8 years ago
I just added tests that demonstrate that AnalysisEngineFactory does the correct 
thing - it allows you to create a description missing mandatory values (so you 
can e.g. write them out to xml as a template) but barfs when you try to create 
an analysis engine.  See 
org.uimafit.factory.AnalysisEngineFactoryTest.testIssue5a() and b.  

Richard, I see your point about the exception.  UIMA complains before intialize 
is ever called on the component.  But your point about there not being enough 
information in the trace speaks to your poor parameter naming!  When I print 
out the stack trace I get:
No value has been assigned to the mandatory configuration parameter 
org.uimafit.factory.testAes.ParameterizedAE.PARAM_FLOAT_3.

Not hard to find that one!

But actually, it does give the component name at the very top of the stack 
trace.  The first line of the stack trace contains:

Error initializing "org.uimafit.factory.testAes.ParameterizedAE" from descriptor

So, there is no task associated with this issue and I think we are in agreement 
about what the behavior should be and we have a test that demonstrates uimaFIT 
is behaving correctly.  Please reopen as you see fit.

Original comment by pvogren@gmail.com on 11 Jun 2010 at 5:30