[
https://issues.apache.org/jira/browse/SOLR-3963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496777#comment-13496777
]
Hoss Man commented on SOLR-3963:
--------------------------------
Hey Bill,
First off, some specific comments on your patch...
* your tests look pretty good to me
* your description and toString methods don't look quite right -- you should be
calling target.description() and target.toString(doc)
** both should also be using defaultVal -- that looks like an oversight in
RangeMapFloatFunction that you copied over
* the class level javadoc comment makes no sense ... also a bug copy/pasted
from RangeMapFloatFunction it seems
* instead of a new "RangeMapFloatFunction2" i think it would make a lot more
sense to just change "RangeMapFloatFunction" to use your new code and modify
the existing constructor to call your new constructor wrapping the constants in
ConstValueSource instances
** while we're at it we can fix that javadoc bug and the oversight of ignoring
defaultVal in description & toString
* if we're going to support ValueSources in target & default, is there any
reason not to support ValueSources for min & max as well?
Second: some broader comments on the overall idea that occured to me while
reading your patch...
The changes you are proposing are definitely more general purpose then the
current implementation -- but the trade off is that (in theory) using constant
values is faster then dealing with nested ValueSource objects because of the
method call overhead. so (in theory) making this change adversely affects
people who are currently using constant values. That doesn't mean it shouldn't
be done -- but it's worthwhile taking a moment to think about wether there is a
best of both worlds situation.
Unless i'm missing something, what you want to do...
{noformat}map(nested(...),min,max,target(...),default(...)){noformat}
...should already possible using something like...
{noformat}if(map(nested(...),min,max,1,0),target(...),default(...)){noformat}
...and that should have roughly the same performance as your your suggested
change, w/o affecting the performance for people who are only using constants.
So perhaps we should actually just leave the code in the {{map(...)}} function
alone, and instead improve it's docs to clarify that for people who want more
complex non-constant values they can use that {{if(...)}} pattern.
what do you think?
> SOLR: map() does not allow passing sub-functions in 4,5 parameters
> ------------------------------------------------------------------
>
> Key: SOLR-3963
> URL: https://issues.apache.org/jira/browse/SOLR-3963
> Project: Solr
> Issue Type: Improvement
> Affects Versions: 4.0
> Reporter: Bill Bell
> Assignee: Hoss Man
> Priority: Minor
> Fix For: 4.0
>
> Attachments: SOLR-3963.2.patch
>
>
> I want to do:
> boost=map(achievement_count,1,1000,recip(achievement_count,-.5,10,25),1)
> I want to return recip(achievement_count,-.5,10,25) if achievement_count is
> between 1 and 1,000. FOr any other values I want to return 1.
> I cannot get it to work. I get the error below. Interesting this does work:
> boost=recip(map(achievement_count,0,0,-200),-.5,10,25)
> It almost appears that map() cannot take a function.
> Specified argument was out of the range of valid values.
> Parameter name: value
> Description: An unhandled exception occurred during the execution of the
> current web request. Please review the stack trace for more information about
> the error and where it originated in the code.
> Exception Details: System.ArgumentOutOfRangeException: Specified argument was
> out of the range of valid values.
> Parameter name: value
> Source Error:
> An unhandled exception was generated during the execution of the current web
> request. Information regarding the origin and location of the exception can
> be identified using the exception stack trace below.
> Stack Trace:
> [ArgumentOutOfRangeException: Specified argument was out of the range of
> valid values.
> Parameter name: value]
> System.Web.HttpResponse.set_StatusDescription(String value) +5200522
> FacilityService.Controllers.FacilityController.ActionCompleted(String
> actionName, IFacilityResults results) +265
>
> FacilityService.Controllers.FacilityController.SearchByPointCompleted(IFacilityResults
> results) +25
> lambda_method(Closure , ControllerBase , Object[] ) +114
> System.Web.Mvc.Async.<>c__DisplayClass7.<BeginExecute>b__5(IAsyncResult
> asyncResult) +283
>
> System.Web.Mvc.Async.<>c__DisplayClass41.<BeginInvokeAsynchronousActionMethod>b__40(IAsyncResult
> asyncResult) +22
>
> System.Web.Mvc.Async.<>c__DisplayClass3b.<BeginInvokeActionMethodWithFilters>b__35()
> +120
>
> System.Web.Mvc.Async.<>c__DisplayClass51.<InvokeActionMethodFilterAsynchronously>b__4b()
> +452
>
> System.Web.Mvc.Async.<>c__DisplayClass39.<BeginInvokeActionMethodWithFilters>b__38(IAsyncResult
> asyncResult) +15
> System.Web.Mvc.Async.<>c__DisplayClass2c.<BeginInvokeAction>b__22() +33
>
> System.Web.Mvc.Async.<>c__DisplayClass27.<BeginInvokeAction>b__24(IAsyncResult
> asyncResult) +240
> System.Web.Mvc.<>c__DisplayClass19.<BeginExecuteCore>b__14(IAsyncResult
> asyncResult) +28
>
> System.Web.Mvc.Async.<>c__DisplayClass4.<MakeVoidDelegate>b__3(IAsyncResult
> ar) +15
> System.Web.Mvc.AsyncController.EndExecuteCore(IAsyncResult asyncResult) +63
>
> System.Web.Mvc.Async.<>c__DisplayClass4.<MakeVoidDelegate>b__3(IAsyncResult
> ar) +15
> System.Web.Mvc.<>c__DisplayClassb.<BeginProcessRequest>b__4(IAsyncResult
> asyncResult) +42
>
> System.Web.Mvc.Async.<>c__DisplayClass4.<MakeVoidDelegate>b__3(IAsyncResult
> ar) +15
> System.Web.CallHandlerExecutionStep.OnAsyncHandlerCompletion(IAsyncResult
> ar) +282
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
