As I said on the call, I prefer the approach of continuing to call the existing stuff "Logic," and calling our new, lightweight interfaces something new, like "Rule". At least in my mind, "logic" is tied to being able to do complex things like "is the last cd4 count in the last 3 months < 350?". Whereas our new approach won't try to touch this, at least for a while--it's just allowing you to execute a defined rule, and get a result. (Maybe others interpret "logic" and "rule" differently.)
I really like the idea of: - introduce the new Rule/RuleProvider interfaces in core (or whatever we call them) - moving the logic code that's currently in core to be completely in the logic module - making the old logic code implement the new Rule/RuleProvider interfaces - logic shouldn't be a "core module" anymore This might be a one-off inconvenience for Chica and AMPATH (they may need to recompile code that relies on existing logic to refer to a new package name, and/or switch Context.getLogicService() to Context.getService(LogicService.class)) but it will be *way* better in the long run (especially for CHICA) to have all logic code in one module that's less dependent on different core versions, and whose development can progress independently of core releases. This is pretty much what we did when we pulled cohort builder, data exports, and xml reports out into the reportingcompatibility module. Doing that was quite successful, allowing PIH and others to get the latest bugfixed versions of cohort builder and data exports on every OpenMRS version from 1.5+ without requiring much from the core development team. The only difficulties were due to us *not* wanting to change any core interfaces, and because we misestimated by a factor of years how long it would take to produce replacements for those functionalities in the reporting module. -Darius On Thu, Nov 17, 2011 at 6:39 AM, Michael Seaton <[email protected]> wrote: > ** > Hi Steve, > > The specifics of this are still being discussed in terms of where the > existing Logic code lives on - but the idea is that it will definitely be > supported somewhere for backwards compatibility. I think two of our > options are: > > - Leave all of the Logic interfaces and classes in core as they > currently are, but deprecate them all, and then do our rewrite in a > different package > - Move all of the Logic interfaces and classes we no longer want to > support in core (including DataSources) into the Logic Module and allow > them to be used from there, as is > > > We need to think through the implications of these options. For true > backwards-compatibility we would likely need to use the first option. But > if we can live with requiring module developers to make some minor code > changes (eg. explicitly include logic as a required module, change calls > from Context.getLogicService() to Context.getService(LogicService.class), > etc. then option 2 might be viable > > Other opinions or options? > > Mike > > > > > On 11/17/2011 08:17 AM, McKee, Steven Jay wrote: > > Is there still going to be a way to get the data sources from logic. We > currently rely on the getLogicDataSource(String name) method in many places > in our code to get a handle on the multiple data sources we use.**** > > ** ** > > Steve**** > > ** ** > > *From:* [email protected] [mailto:[email protected] <[email protected]>] *On > Behalf Of *Michael Seaton > *Sent:* Wednesday, November 16, 2011 5:15 PM > *To:* [email protected] > *Subject:* Re: [OPENMRS-DEV] The future of Logic**** > > ** ** > > 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 > **** > ------------------------------ > 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]
