Perhaps we should revisit this on the design call in 2 weeks? -Darius
On Wed, Nov 16, 2011 at 2:14 PM, Michael Seaton <[email protected]> wrote: > ** > Hi all, > > Thanks to everyone who participated in this call today - I think we made a > lot of progress. Here is a summary of what we were able to produce (full > etherpad notes here <http://notes.openmrs.org/Design-Forum-2011-11-16>): > * > Requirements/Goals* > > - Existing logic engine can be implemented as an optional module > without too much trouble. > - A shared Rule interface > - Rules provide a method to evaluate within a context + parameters > - A shared Result interface > - Result declares it's type explicitly > - Provides a mechanism for consumers to easily distinguish between > & use lists vs. single values > - Provides an easy way to coerce results between different types and > between lists vs. single value > - A shared LogicContext interface within which rules are evaluated > - Centralized token registration by providing token, rule ID, > provider, and configuration (unique across providers) > - Support for parameters. > - Caching is deferred to rule evaluators for now > > *Skeleton Interfaces / Implementations* > > *Rule:* > > interface *Rule* { > public Set<RuleParameterInfo> getParameterList(); // TODO: We didn't > discuss this interface, but we did agree that rules need to support > parameters > } > > interface *RuleEvaluator* { > boolean canEvaluate(Rule rule); // TODO: This might be better > implemented through annotations. Needs further discussion > Map<Integer, Result> evaluate(Cohort, Rule, Map<String, Object>, > RuleContext); // TODO: We probably want to wrap Map<Integer, Result> in a > proper class > } > > interface *RuleProvider* { > Rule getRuleInstance(String ruleName, String extraConfig); // ruleName > could be anything the provider wants. might typically be a classname. > } > > interface / implementation *RuleService*/*RuleServiceImpl* { > RuleContext createContext(); // Ensures that RuleContext can be > overridden as needed > void registerToken(String token, RuleProvider provider, String ruleName, > String extraConfig); > void unregisterToken(String token, RuleProvider provider); // provider > for safety > > (the below methods might also have implementations without RuleContext > and/or without params for convenience) > Result evaluate(Integer ptId, String token, Map<String, Object> params, > RuleContext context) { > // create a cohort with a single patient, call the evaluate method on > the cohort, return the result for that patient > } > Map<Integer, Result> evaluate(Cohort c, String token, Map<String, > Object> params, RuleContext context) { > // find the appropriate RuleProvider, ruleName, extraConfig for the > given token; get the Rule from the RuleProvider passing in the ruleName and > extraConfig; call evaluate method on the Rule > } > Result evaluate(Integer ptId, Rule rule, Map<String, Object> params, > RuleContext context) { > // construct a cohort of one; get the evaluator for the passed rule; > call evaluate passing in the cohort, params and the context; return the > result for the patient > } > Map<Integer, Result> evaluate(Cohort c, Rule rule, Map<String, Object> > params, RuleContext context) { > // get the evaluator for the passed rule; call evaluate passing in the > cohort, params and the context; return the result > } > } > > interface *RuleContext* { > // TODO: Figure out whether this has indexDate, any caching, etc. > } > > *Result:* > > interface Result { > public Object getValue(); > public Date getDatetime(); // Needs more discussion if this is > appropriate on the base interface (eg. what to do for Lists) > public String formatAsString(); > } > > class NumericResult { > private Double result; > private Date datetime; > public Object getValue() { return result; } > public Date getDatetime() { return datetime; } > } > > class ListResult { > private List<Result> results; > public Object getValue() { return results; } > public Date getDatetime() { // Not sure what to do here. Get the > datetime of the first result? Maybe this method doesn't belong > } > > class ObsResult { > private Obs obs; > public getValue() { return obs; } > public Date getDatetime() { return obs.getObsDatetime(); } > } > > *Use of Result:* > > For coercing / converting Results to scalars for use in comparisons etc, > for example: > if (eval("BMI") > 23) { ... }, we discussed a few possible approaches: (TODO: > Decide on one) > > 1. adding methods like "asDouble()", "asDate()", "asString()", > "asBoolean()" to the Result interface (as we have now) > 2. Add single method to Result like: eval("BMI").coerce(Double.class) > 23 > 3. Add utility method like: LogicUtil.toDouble(eval("BMI")) > 23 > 4. Add utility method like: LogicUtil.coerce(eval("BMI"), Double.class) > > 23 > 5. Add a variety of converters like: new > DoubleConverter().convert(eval("BMI")) > 23 > > Benefit of #5 is that it is cleaner than a massive utility method with > lots of conditional logic, and that it allows modules to plug new > converters into the framework as needed. It might look something like this: > > interface ResultConverter<T> { > public T convert(Result); > } > > class DoubleConverter<Double> { > public Double convert(Result result) { return > Double.valueOf(result.toString()); } > } > > > We ran out of time before we could really discuss next steps on all of > this. Should we carve out time in another design forum in December? > > Thanks, > Mike > > > > > > On 11/16/2011 03:23 AM, Ben Wolfe wrote: > > Lets plan on having a discussion today about this on the design call at > 2pm. We can write up the solutions (and new questions) for Tammy to read > offline. > > Ben > > On Tue, Nov 15, 2011 at 2:21 PM, Dugan, Tammy Marie <[email protected]>wrote: > >> I am on maternity leave and have my hands pretty full with a two week old >> and a two year old. If you have any specific questions, please feel free to >> email me. Email is a lot easier for me right now than phone. >> >> Thanks, >> >> Tammy Dugan >> >> Quoting Michael Seaton <[email protected]>: >> >> Yes, I will be there and looking forward to talking about this. >>> >>> Mike >>> >>> From: [email protected] [mailto:[email protected]] On Behalf Of Ben Wolfe >>> Sent: Tuesday, November 15, 2011 3:14 AM >>> To: [email protected] >>> Subject: Re: [OPENMRS-DEV] The future of Logic >>> >>> Would it be possible for folks to join tomorrow the 16th at 2pm EST? >>> I fear that some people will be missing next Wednesday due to the US >>> holiday on Thursday the 24th. >>> >>> Tammy? Dave? Mike? Darius? Burke? Roger? Win? Beuller? >>> >>> https://wiki.openmrs.org/display/RES/Design+Forum >>> >>> Ben >>> On Mon, Nov 14, 2011 at 8:55 PM, Burke Mamlin >>> <[email protected]<mailto:[email protected]>> wrote: >>> Can we enumerate what we want from the simplified logic service? >>> >>> * Existing logic engine can be implemented as an optional module >>> without too much trouble. >>> * Strongly instead of loosely typed results. >>> * Support for parameters. >>> * ... >>> >>> Do we want to support an index date (i.e., avoid assuming today's date)? >>> >>> Don't we want to forego tokens? Or are we assuming that consumers of >>> logic know which rule provider to consult (which may or may not use >>> tokens)? If the consumer needs to know which provider to consult and >>> that provider will presumably be handling the evaluation, then what >>> purpose does the logic service serve? Alternatively, rule providers >>> could be registered with the logic service and then consumers could >>> come to the logic service with a token (i.e., not need to know the >>> provider). >>> >>> I'm assuming that we'll defer caching & criteria implementations to >>> the various forms of logic (at least for now). >>> >>> What's the purpose of LogicContext now? In the original design, >>> LogicContext served a few purposes: (1) proxy for all data requests >>> by rules, (2) provide a context for evaluations ? index date & >>> >>> parameters ? that could be stacked for recursion, (3) context for >>> caching results, and (4) an abstraction so that the same rule could >>> be run against a patient or cohort. If we aren't using logic context >>> for any of these, do we need to maintain LogicContext at this stage? >>> >>> Looking forward to the design call. >>> >>> Cheers, >>> >>> -Burke >>> >>> On Wed, Nov 9, 2011 at 10:23 PM, Michael Seaton >>> <[email protected]<mailto:[email protected]>> wrote: >>> Hi all, >>> >>> I wanted to pick up a thread on the future of Logic that we spent >>> some time discussing during the Developers Forum on October >>> 27<https://wiki.openmrs.org/x/5oamAQ>. >>> >>> >>> The overall consensus on the call was that "Logic is too complicated. >>> We wish there were a simpler implementation, that were easier to >>> adapt to individual needs. We wish it were more rigorously tested, >>> including performance testing". >>> >>> Anyone that cares about preserving Logic as it currently exists, or >>> wants to see it change in a specific way, please read further >>> >>> Specific users of Logic have communicated the following needs: >>> >>> * Tammy and her team require more control over the current Logic >>> >>> implementation, and better testing around it, so that future upgrades >>> do not cause serious bugs and performance degradation >>> * Win and Tammy need implementations of Logic that are optimized >>> >>> for running lots of rules for individual patients at a time >>> * Mike needs an implementation of Logic that is optimized for >>> >>> running a single rule for lots of patients at a time as well >>> * Mike wants to be able to choose to not use the existing Logic >>> >>> implementation in favor of an implementation provided by the >>> Reporting module >>> To accomplish this, the following high-level refactoring steps are >>> being considered: >>> >>> * Reduce and simplify the number of interfaces and interface >>> >>> methods that Logic exposes in OpenMRS core >>> * De-couple the existing Logic Module in such a way that it is one >>> >>> of many possible "Rule Providers" that can plug into this >>> lighter-weight core framework >>> * Remove, if possible and practical, the need to have a "required" >>> >>> Logic module installed >>> Specifically, we are considering an approach that would reduce the >>> core Logic interfaces to something like: >>> interface LogicContext; >>> >>> interface Result; (classes DateResult, NumberResult, ListResult...) >>> >>> interface Rule { >>> Set<RuleParameterInfo> getParameterInfo(); >>> Class<? extends Result> getReturnType(); >>> Result evaluate(Integer patientId, Map<String, Object> parameters, >>> LogicContext context); >>> } >>> >>> interface LogicService { >>> public Result evaluate(Rule rule, Patient patient, Map<String, >>> Object> parameters, LogicContext context); >>> public Map<Integer, Result> evaluate(Rule rule, Cohort cohort, >>> Map<String, Object> parameters, LogicContext context); >>> } >>> I would like for this to spur the following potential activities: >>> >>> * Give anyone who has not yet been aware of these discussions, or >>> >>> who does not agree with this approach, an opportunity to weigh in and >>> get involved in the process >>> * Make a plan to put this topic onto one or more future Design >>> >>> Forum calls, in order to agree upon the revised design, make tickets, >>> and establish an owner for moving it forward. >>> * Publicize when this Design Forum will occur so that interested >>> >>> parties can be involved as desired >>> Thanks! >>> Mike >>> >>> >>> ________________________________ >>> Click here to >>> >>> unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> >>> from OpenMRS Developers' mailing >>> >>> list >>> >>> ________________________________ >>> Click here to >>> >>> unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> >>> from OpenMRS Developers' mailing >>> >>> list >>> >>> ________________________________ >>> Click here to >>> >>> unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> >>> from OpenMRS Developers' mailing >>> list >>> >>> _________________________________________ >>> >>> To unsubscribe from OpenMRS Developers' mailing list, send an e-mail >>> to [email protected] with "SIGNOFF openmrs-devel-l" in the >>> body (not the subject) of your e-mail. >>> >>> [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l] >>> >>> >> _________________________________________ >> >> To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to >> [email protected] with "SIGNOFF openmrs-devel-l" in the body >> (not the subject) of your e-mail. >> >> [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l] >> > > ------------------------------ > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > > ------------------------------ > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
