You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "alessandrobenedetti (via GitHub)" <gi...@apache.org> on 2025/01/31 17:20:45 UTC
[PR] SOLR-17632: Text to Vector Update Request Processor [solr]
alessandrobenedetti opened a new pull request, #3151:
URL: https://github.com/apache/solr/pull/3151
https://issues.apache.org/jira/browse/SOLR-17632
# Description
Scope of this issue is to introduce support for automatic text vectorisation in Apache Solr.
A LLM fine-tuned for sentence similarity will be accessed to vectorise the text.
Apache Solr hosts the configuration parameters to access vectorision.
# Solution
An Update Request Processor that access the managed resource and call the vectorisation service for each document to process.
# Tests
Unit and integration tests have been added
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943565203
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
Review Comment:
1) @cpoerschke : I double checked and the langchain4j library 'embed' method (that's used in our 'vectorise' method), doesn't return any exception, but I gree we should investigate what happens if that request fails (my best guess is we get an empty vector or null, I'll add that to tests)
2) @epugh : given that 'update.chain' is a parameter, if you configure a chain with no vector enrichment and a chain with vector enrichment, what prevents you from first index using the 'no vectors' chain and then slowly updating the index with atomic updates that add vectors (using the vector-chain)? We should double check and add to the documentation once we consolidate the code, what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943595213
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ if (args != null) {
+ params = args.toSolrParams();
+ inputField = params.get(INPUT_FIELD_PARAM);
+ checkNotNull(INPUT_FIELD_PARAM, inputField);
+
+ outputField = params.get(OUTPUT_FIELD_PARAM);
+ checkNotNull(OUTPUT_FIELD_PARAM, outputField);
+
+ modelName = params.get(MODEL_NAME);
+ checkNotNull(MODEL_NAME, modelName);
Review Comment:
much cleaner, thanks David!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937657567
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
Review Comment:
Wondering re: `textToVector` as class member vs. local variable.
edit: if `model` is configured i.e. always the same it could be a class member, perhaps, but if perhaps `model` was a field in the `cmd` document then it would need to be a local variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1942178207
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ if (args != null) {
Review Comment:
args should never be null
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ if (args != null) {
+ params = args.toSolrParams();
+ inputField = params.get(INPUT_FIELD_PARAM);
+ checkNotNull(INPUT_FIELD_PARAM, inputField);
+
+ outputField = params.get(OUTPUT_FIELD_PARAM);
+ checkNotNull(OUTPUT_FIELD_PARAM, outputField);
+
+ modelName = params.get(MODEL_NAME);
+ checkNotNull(MODEL_NAME, modelName);
+ }
+ }
+
+ private void checkNotNull(String paramName, Object param) {
+ if (param == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Text to Vector UpdateProcessor '" + paramName + "' can not be null");
+ }
+ }
+
+ @Override
+ public UpdateRequestProcessor getInstance(SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor next) {
+ req.getCore().getLatestSchema().getField(inputField);
Review Comment:
What's the point of this line?
##########
solr/modules/llm/src/test-files/solr/collection1/conf/solrconfig-llm-indexing-notDenseVectorField.xml:
##########
Review Comment:
Do you really need new test configuration files? Could you combine this into another in this module?
I challenge devs on this; test config files are a pain to maintain.
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ if (args != null) {
+ params = args.toSolrParams();
+ inputField = params.get(INPUT_FIELD_PARAM);
+ checkNotNull(INPUT_FIELD_PARAM, inputField);
+
+ outputField = params.get(OUTPUT_FIELD_PARAM);
+ checkNotNull(OUTPUT_FIELD_PARAM, outputField);
+
+ modelName = params.get(MODEL_NAME);
+ checkNotNull(MODEL_NAME, modelName);
Review Comment:
see `params.required()` for concise mandatory checking
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943616272
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
Review Comment:
I agree, texttovector is horribly unreadable, maybe 'textvectorisation' ? adding it in the coming commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #3151:
URL: https://github.com/apache/solr/pull/3151#issuecomment-2627858272
Let's give it a first round of discussion/brainstorming/improvements.
Then I'll adjust the gradle check, documentation and changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943488899
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943278670
##########
solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java:
##########
@@ -88,13 +88,33 @@ private static void checkUpdateU(String message, String update, boolean shouldSu
if (response != null) fail(m + "update was not successful: " + response);
} else {
String response = restTestHarness.validateErrorUpdate(update);
- if (response != null) fail(m + "update succeeded, but should have failed: " + response);
+ if (response == null) fail(m + "update succeeded, but should have failed: " + response);
}
} catch (SAXException e) {
throw new RuntimeException("Invalid XML", e);
}
}
+ public static void checkUpdateU(String update, String... tests) {
Review Comment:
Not specific per se to this, but I wish we had a clearer plan about the future of RestTestBase. ARe we embracing it?
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
Review Comment:
I was chatting with @iamsanjay this morning, and I was expounding on the thought that a lot of folks might want to first index the document with just the core text/string/numbers, and then, since enrichment is SLOW, come back with a streaming expression and do things like vectorization, and an atomic update.. that way you pump your data in as fast as possible, and then enrich at your leisure... This model constrains your indexing speed to your vectorization speed. Not saying we should have the update request processor approach, but I also want to see a "out of band" vectorization process.
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
Review Comment:
`texttovector` looks odd.. and I know a. package like `text.to.vector` is silly. Isn't this an embedding tool? text to vector is making an embedding right? or... `llm.vectorization.update.processor`?
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactoryTest.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.MultiMapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class TextToVectorUpdateProcessorFactoryTest extends TestLlmBase {
+ private TextToVectorUpdateProcessorFactory factoryToTest =
+ new TextToVectorUpdateProcessorFactory();
+ private NamedList<String> args = new NamedList<>();
+
+ @BeforeClass
+ public static void initArgs() throws Exception {
+ setupTest("solrconfig-llm.xml", "schema.xml", false, false);
+ }
+
+ @AfterClass
+ public static void after() throws Exception {
+ afterTest();
+ }
+
+ @Test
+ public void init_fullArgs_shouldInitFullClassificationParams() {
+ args.add("inputField", "_text_");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+ factoryToTest.init(args);
+
+ assertEquals("_text_", factoryToTest.getInputField());
+ assertEquals("vector", factoryToTest.getOutputField());
+ assertEquals("model1", factoryToTest.getModelName());
+ }
+
+ @Test
+ public void init_nullInputField_shouldThrowExceptionWithDetailedMessage() {
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Text to Vector UpdateProcessor 'inputField' can not be null", e.getMessage());
+ }
+
+ @Test
+ public void init_notExistentInputField_shouldThrowExceptionWithDetailedMessage() throws Exception {
+ args.add("inputField", "notExistentInput");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ Map<String, String[]> params = new HashMap<>();
+ MultiMapSolrParams mmparams = new MultiMapSolrParams(params);
+ SolrQueryRequestBase req = new SolrQueryRequestBase(solrClientTestRule.getCoreContainer().getCore("collection1"), (SolrParams) mmparams) {};
Review Comment:
what is the trailing `{}`? not java I am familiar with!
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
+ List<Float> vectorAsList = new ArrayList<Float>(vector.length);
+ for (float f : vector) {
+ vectorAsList.add(f);
+ }
+ doc.addField(outputField, vectorAsList);
+ }
+ super.processAdd(cmd);
+ }
+
+ protected boolean isNullOrEmpty(SolrInputField inputFieldContent, SolrInputDocument doc, String fieldName) {
Review Comment:
do we warn on these types of "data model" issues in other aspects of solr? I mean, who would ever check solr logs to spot that these input fields are missing? I see this cluttering the logs and not being very useful to have operationally.. I think I would just query solr "show me all docs that don't have the field populate" to find the issue....
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TextToVectorUpdateProcessorTest extends TestLlmBase {
+
+ @BeforeClass
+ public static void init() throws Exception {
+ setupTest("solrconfig-llm-indexing.xml", "schema.xml", false, false);
+
+ }
+
+ @Test
+ public void processAdd_inputField_shouldVectoriseInputField()
+ throws Exception {
+ loadModel("dummy-model.json");
+ assertU(adoc("id", "99", "_text_", "Vegeta is the saiyan prince."));
+ assertU(adoc("id", "98", "_text_", "Vegeta is the saiyan prince."));
+ assertU(commit());
+
+ final String solrQuery = "*:*";
+ final SolrQuery query = new SolrQuery();
+ query.setQuery(solrQuery);
+ query.add("fl", "id,vector");
+
+ assertJQ(
+ "/query" + query.toQueryString(),
+ "/response/numFound==2]",
+ "/response/docs/[0]/id=='99'",
+ "/response/docs/[0]/vector==[1.0, 2.0, 3.0, 4.0]",
+ "/response/docs/[1]/id=='98'",
+ "/response/docs/[1]/vector==[1.0, 2.0, 3.0, 4.0]");
+
+ restTestHarness.delete(ManagedTextToVectorModelStore.REST_END_POINT + "/dummy-1");
+ }
+
+ /*
+ This test looks for the 'dummy-1' model, but such model is not loaded, the model store is empty, so the update fails
+ */
+ @Test
+ public void processAdd_modelNotFound_shouldRaiseException() {
+ assertFailedU("This update should fail but actually succeeded", adoc("id", "99", "_text_", "Vegeta is the saiyan prince."));
+
+ checkUpdateU(adoc("id", "99", "_text_", "Vegeta is the saiyan prince."),
+ "/response/lst[@name='error']/str[@name='msg']=\"The model requested 'dummy-1' can't be found in the store: /schema/text-to-vector-model-store\"",
+ "/response/lst[@name='error']/int[@name='code']='400'");
+ }
+
+ @Test
+ public void processAdd_emptyInputField_shouldLogAndIndexWithNoVector() throws Exception {
+ loadModel("dummy-model.json");
+ assertU(adoc("id", "99", "_text_", ""));
+ assertU(adoc("id", "98", "_text_", "Vegeta is the saiyan prince."));
+ assertU(commit());
+
+ final String solrQuery = "*:*";
+ final SolrQuery query = new SolrQuery();
+ query.setQuery(solrQuery);
+ query.add("fl", "id,vector");
+
+ assertJQ(
+ "/query" + query.toQueryString(),
+ "/response/numFound==2]",
+ "/response/docs/[0]/id=='99'",
+ "!/response/docs/[0]/vector==", //no vector field for the document 99
Review Comment:
love the comment!
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ if (args != null) {
Review Comment:
Yay! I hate the null checks we have everywhere, they make me nervous!
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TextToVectorUpdateProcessorTest extends TestLlmBase {
+
+ @BeforeClass
+ public static void init() throws Exception {
+ setupTest("solrconfig-llm-indexing.xml", "schema.xml", false, false);
+
+ }
+
+ @Test
+ public void processAdd_inputField_shouldVectoriseInputField()
+ throws Exception {
+ loadModel("dummy-model.json");
+ assertU(adoc("id", "99", "_text_", "Vegeta is the saiyan prince."));
+ assertU(adoc("id", "98", "_text_", "Vegeta is the saiyan prince."));
+ assertU(commit());
+
+ final String solrQuery = "*:*";
+ final SolrQuery query = new SolrQuery();
+ query.setQuery(solrQuery);
+ query.add("fl", "id,vector");
+
+ assertJQ(
+ "/query" + query.toQueryString(),
+ "/response/numFound==2]",
+ "/response/docs/[0]/id=='99'",
+ "/response/docs/[0]/vector==[1.0, 2.0, 3.0, 4.0]",
+ "/response/docs/[1]/id=='98'",
+ "/response/docs/[1]/vector==[1.0, 2.0, 3.0, 4.0]");
+
+ restTestHarness.delete(ManagedTextToVectorModelStore.REST_END_POINT + "/dummy-1");
Review Comment:
does it belong in the test below actually?
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TextToVectorUpdateProcessorTest extends TestLlmBase {
+
+ @BeforeClass
+ public static void init() throws Exception {
+ setupTest("solrconfig-llm-indexing.xml", "schema.xml", false, false);
+
+ }
+
+ @Test
+ public void processAdd_inputField_shouldVectoriseInputField()
+ throws Exception {
+ loadModel("dummy-model.json");
+ assertU(adoc("id", "99", "_text_", "Vegeta is the saiyan prince."));
+ assertU(adoc("id", "98", "_text_", "Vegeta is the saiyan prince."));
+ assertU(commit());
+
+ final String solrQuery = "*:*";
+ final SolrQuery query = new SolrQuery();
+ query.setQuery(solrQuery);
+ query.add("fl", "id,vector");
+
+ assertJQ(
+ "/query" + query.toQueryString(),
+ "/response/numFound==2]",
+ "/response/docs/[0]/id=='99'",
+ "/response/docs/[0]/vector==[1.0, 2.0, 3.0, 4.0]",
+ "/response/docs/[1]/id=='98'",
+ "/response/docs/[1]/vector==[1.0, 2.0, 3.0, 4.0]");
+
+ restTestHarness.delete(ManagedTextToVectorModelStore.REST_END_POINT + "/dummy-1");
Review Comment:
is this dummy-1 call needed? if it's a test, shouldn't there be an assert? if it's clean up, maybe make it clear?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943565203
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
Review Comment:
1) @cpoerschke : I double checked and the langchain4j library 'embed' method (that's used in our 'vectorise' method) returns a RuntimeException .
That's bad as it was not detected without investigating the internals of the code (I hate these practices).
I'll give it a thought, any suggestion is welcome!
2) @epugh : given that 'update.chain' is a parameter, if you configure a chain with no vector enrichment and a chain with vector enrichment, what prevents you from first index using the 'no vectors' chain and then slowly updating the index with atomic updates that add vectors (using the vector-chain)? We should double check and add to the documentation once we consolidate the code, what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943614530
##########
solr/modules/llm/src/test-files/solr/collection1/conf/solrconfig-llm-indexing-notDenseVectorField.xml:
##########
Review Comment:
I could, but the reason I added it is that I struggled to find testing methods such as org.apache.solr.util.RestTestBase#assertU(java.lang.String) that takes the chain as a parameter.
So I added as the default and I could test it.
I would not want to be the default when indexing docs for the query time test.
If you have any suggestion I'm open to changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1944799990
##########
solr/modules/llm/src/java/org/apache/solr/llm/textvectorisation/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.textvectorisation.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.RequiredSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.textvectorisation.model.SolrTextToVectorModel;
+import org.apache.solr.llm.textvectorisation.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ private String inputField;
+ private String outputField;
+ private String modelName;
+ private SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ params = args.toSolrParams();
+ RequiredSolrParams required = params.required();
+ inputField = required.get(INPUT_FIELD_PARAM);
+ outputField = required.get(OUTPUT_FIELD_PARAM);
+ modelName = required.get(MODEL_NAME);
+ }
+
+ @Override
+ public UpdateRequestProcessor getInstance(SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor next) {
+ IndexSchema latestSchema = req.getCore().getLatestSchema();
+ if(!latestSchema.hasExplicitField(inputField)){
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "undefined field: \"" + inputField + "\"");
+ }
+ if(!latestSchema.hasExplicitField(outputField)){
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "undefined field: \"" + outputField + "\"");
+ }
+
+ final SchemaField outputFieldSchema = latestSchema.getField(outputField);
+ assertIsDenseVectorField(outputFieldSchema);
+
+ ManagedTextToVectorModelStore modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ SolrTextToVectorModel textToVector = modelStore.getModel(modelName);
Review Comment:
I presume looking up the model is fast/cached if it already exists?
##########
solr/modules/llm/src/test/org/apache/solr/llm/textvectorisation/search/TextToVectorQParserTest.java:
##########
@@ -29,6 +30,11 @@ public static void init() throws Exception {
loadModel("dummy-model.json");
}
+ @AfterClass
+ public static void cleanup() throws Exception {
+ afterTest();
+ }
Review Comment:
weird; why? Weird to see a test cleanup also be a suite cleanup.
(same for TestModelManager)
##########
solr/modules/llm/src/test/org/apache/solr/llm/textvectorisation/update/processor/TextToVectorUpdateProcessorFactoryTest.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.textvectorisation.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.MultiMapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class TextToVectorUpdateProcessorFactoryTest extends TestLlmBase {
+ private TextToVectorUpdateProcessorFactory factoryToTest =
+ new TextToVectorUpdateProcessorFactory();
+ private NamedList<String> args = new NamedList<>();
+
+ @BeforeClass
+ public static void initArgs() throws Exception {
+ setupTest("solrconfig-llm.xml", "schema.xml", false, false);
+ }
+
+ @AfterClass
+ public static void after() throws Exception {
+ afterTest();
+ }
+
+ @Test
+ public void init_fullArgs_shouldInitAllParams() {
+ args.add("inputField", "_text_");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+ factoryToTest.init(args);
+
+ assertEquals("_text_", factoryToTest.getInputField());
+ assertEquals("vector", factoryToTest.getOutputField());
+ assertEquals("model1", factoryToTest.getModelName());
+ }
+
+ @Test
+ public void init_nullInputField_shouldThrowExceptionWithDetailedMessage() {
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Missing required parameter: inputField", e.getMessage());
+ }
+
+ @Test
+ public void init_nullOutputField_shouldThrowExceptionWithDetailedMessage() {
+ args.add("inputField", "_text_");
+ args.add("model", "model1");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Missing required parameter: outputField", e.getMessage());
+ }
+
+ @Test
+ public void init_nullModel_shouldThrowExceptionWithDetailedMessage() {
+ args.add("inputField", "_text_");
+ args.add("outputField", "vector");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Missing required parameter: model", e.getMessage());
+ }
+
+ /* Following tests depends on a real solr schema */
+ @Test
+ public void init_notExistentOutputField_shouldThrowExceptionWithDetailedMessage() throws Exception {
+ args.add("inputField", "_text_");
+ args.add("outputField", "notExistentOutput");
+ args.add("model", "model1");
+
+ Map<String, String[]> params = new HashMap<>();
+ MultiMapSolrParams mmparams = new MultiMapSolrParams(params);
Review Comment:
This is more complex than needed; just create a ModifiableSolrParams (one line). Soon after I merge a PR up for review, you would do `SolrParams.of()`.
##########
solr/modules/llm/src/test/org/apache/solr/llm/textvectorisation/update/processor/TextToVectorUpdateProcessorFactoryTest.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.textvectorisation.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.MultiMapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class TextToVectorUpdateProcessorFactoryTest extends TestLlmBase {
+ private TextToVectorUpdateProcessorFactory factoryToTest =
+ new TextToVectorUpdateProcessorFactory();
+ private NamedList<String> args = new NamedList<>();
Review Comment:
I think it's awkward to have this as a field, as you need to tend to its lifecycle by cleaning. It's a lightweight trivial object so why not have the test classes initialize them?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1964081310
##########
solr/modules/llm/src/java/org/apache/solr/llm/textvectorisation/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.textvectorisation.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.RequiredSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.textvectorisation.model.SolrTextToVectorModel;
+import org.apache.solr.llm.textvectorisation.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ private String inputField;
+ private String outputField;
+ private String modelName;
+ private SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ params = args.toSolrParams();
+ RequiredSolrParams required = params.required();
+ inputField = required.get(INPUT_FIELD_PARAM);
+ outputField = required.get(OUTPUT_FIELD_PARAM);
+ modelName = required.get(MODEL_NAME);
+ }
+
+ @Override
+ public UpdateRequestProcessor getInstance(SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor next) {
+ IndexSchema latestSchema = req.getCore().getLatestSchema();
+ if(!latestSchema.hasExplicitField(inputField)){
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "undefined field: \"" + inputField + "\"");
+ }
+ if(!latestSchema.hasExplicitField(outputField)){
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "undefined field: \"" + outputField + "\"");
+ }
+
+ final SchemaField outputFieldSchema = latestSchema.getField(outputField);
+ assertIsDenseVectorField(outputFieldSchema);
+
+ ManagedTextToVectorModelStore modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ SolrTextToVectorModel textToVector = modelStore.getModel(modelName);
Review Comment:
I have the same assumption, I debugged it a few times and the manage resource keeps track of models in a map so should work as expected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937657567
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
Review Comment:
Wondering re: `textToVector` as class member vs. local variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti merged PR #3151:
URL: https://github.com/apache/solr/pull/3151
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937653570
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
Review Comment:
```suggestion
private final ManagedTextToVectorModelStore modelStore;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943633074
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactoryTest.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.MultiMapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class TextToVectorUpdateProcessorFactoryTest extends TestLlmBase {
+ private TextToVectorUpdateProcessorFactory factoryToTest =
+ new TextToVectorUpdateProcessorFactory();
+ private NamedList<String> args = new NamedList<>();
+
+ @BeforeClass
+ public static void initArgs() throws Exception {
+ setupTest("solrconfig-llm.xml", "schema.xml", false, false);
+ }
+
+ @AfterClass
+ public static void after() throws Exception {
+ afterTest();
+ }
+
+ @Test
+ public void init_fullArgs_shouldInitFullClassificationParams() {
+ args.add("inputField", "_text_");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+ factoryToTest.init(args);
+
+ assertEquals("_text_", factoryToTest.getInputField());
+ assertEquals("vector", factoryToTest.getOutputField());
+ assertEquals("model1", factoryToTest.getModelName());
+ }
+
+ @Test
+ public void init_nullInputField_shouldThrowExceptionWithDetailedMessage() {
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Text to Vector UpdateProcessor 'inputField' can not be null", e.getMessage());
+ }
+
+ @Test
+ public void init_notExistentInputField_shouldThrowExceptionWithDetailedMessage() throws Exception {
+ args.add("inputField", "notExistentInput");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ Map<String, String[]> params = new HashMap<>();
+ MultiMapSolrParams mmparams = new MultiMapSolrParams(params);
+ SolrQueryRequestBase req = new SolrQueryRequestBase(solrClientTestRule.getCoreContainer().getCore("collection1"), (SolrParams) mmparams) {};
Review Comment:
I admit I don't know, I'm not that java savvy, I suspect it has to do with instatiating a subclass of an abstract class?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943628396
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
+ List<Float> vectorAsList = new ArrayList<Float>(vector.length);
+ for (float f : vector) {
+ vectorAsList.add(f);
+ }
+ doc.addField(outputField, vectorAsList);
+ }
+ super.processAdd(cmd);
+ }
+
+ protected boolean isNullOrEmpty(SolrInputField inputFieldContent, SolrInputDocument doc, String fieldName) {
Review Comment:
mmm I see your point, better if we just log a warning say "vectorisation failed", with the reason "null or empty source field" ?
I suspect that silent failure would be equally problematic to understand why there are no vectors? (I also just discovered that the 'vectorise' method could throw runtime exception)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937732098
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
Review Comment:
So text-to-vector in the search case is ephemeral i.e. in case of errors or timeouts the user gets an error or exception back and they may or may not choose to retry.
For text-to-vector in the update case, might some users prefer to index only documents with (all the) vectors and others would rather have the document indexed even if some vectors are missing? (I assume but don't know for sure that the `vectorise` call might throw an exception in certain circumstances and if it's not caught that might fail the indexing for the entire document.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1937651836
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
Review Comment:
```suggestion
private String inputField;
private String outputField;
private String modelName;
private SolrParams params;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943565203
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
Review Comment:
1) @cpoerschke : I double checked and the langchain4j library 'embed' method (that's used in our 'vecctorise' method), doesn't return any exception, but I gree we should investigate what happens if that request fails (my best guess is we get an empty vector or null, I'll add that to tests)
2) @epugh : given that 'update.chain' is a parameter, if you configure a chain with no vector enrichment and a chain with vector enrichment, what prevents you from first index using the 'no vectors' chain and then slowly updating the index with atomic updates that add vectors (using the vector-chain)? We should double check and add to the documentation once we consolidate the code, what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #3151:
URL: https://github.com/apache/solr/pull/3151#issuecomment-2637867724
> I wanted to follow-up on my feedback to the LLM module concerning use of the word "embedding". I first tried to say that I was not familiar with the word, and your response was to remove it (completely?) from the module. If "embedding" is an appropriate word then use it. The documentation should reference it in the ref guide, even if just an "AKA".
Embedding is widely used in the field, but it's a bit ambiguous and to be honest, I'm with you in not using any term that can cause confusion. Do you mean I added embedding in here somewhere? If that's the case, It's a mistake, point it to me and I'll remove it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943608528
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ if (args != null) {
+ params = args.toSolrParams();
+ inputField = params.get(INPUT_FIELD_PARAM);
+ checkNotNull(INPUT_FIELD_PARAM, inputField);
+
+ outputField = params.get(OUTPUT_FIELD_PARAM);
+ checkNotNull(OUTPUT_FIELD_PARAM, outputField);
+
+ modelName = params.get(MODEL_NAME);
+ checkNotNull(MODEL_NAME, modelName);
+ }
+ }
+
+ private void checkNotNull(String paramName, Object param) {
+ if (param == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Text to Vector UpdateProcessor '" + paramName + "' can not be null");
+ }
+ }
+
+ @Override
+ public UpdateRequestProcessor getInstance(SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor next) {
+ req.getCore().getLatestSchema().getField(inputField);
Review Comment:
it checks that 'inputField' is defined in the schema.
With the latest commit I changed it to make it more explicit but I am open to suggestions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943491138
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
Review Comment:
Sure!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943577028
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactory.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.DenseVectorField;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
+
+/**
+ * This class implements an UpdateProcessorFactory for the Text To Vector Update Processor.
+ */
+public class TextToVectorUpdateProcessorFactory extends UpdateRequestProcessorFactory {
+ private static final String INPUT_FIELD_PARAM = "inputField";
+ private static final String OUTPUT_FIELD_PARAM = "outputField";
+ private static final String MODEL_NAME = "model";
+
+ String inputField;
+ String outputField;
+ String modelName;
+ SolrParams params;
+
+
+ @Override
+ public void init(final NamedList<?> args) {
+ if (args != null) {
Review Comment:
my bad, I took inspiration from an old factory, I'll remove this useless check in the next commit!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943636345
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TextToVectorUpdateProcessorTest extends TestLlmBase {
+
+ @BeforeClass
+ public static void init() throws Exception {
+ setupTest("solrconfig-llm-indexing.xml", "schema.xml", false, false);
+
+ }
+
+ @Test
+ public void processAdd_inputField_shouldVectoriseInputField()
+ throws Exception {
+ loadModel("dummy-model.json");
+ assertU(adoc("id", "99", "_text_", "Vegeta is the saiyan prince."));
+ assertU(adoc("id", "98", "_text_", "Vegeta is the saiyan prince."));
+ assertU(commit());
+
+ final String solrQuery = "*:*";
+ final SolrQuery query = new SolrQuery();
+ query.setQuery(solrQuery);
+ query.add("fl", "id,vector");
+
+ assertJQ(
+ "/query" + query.toQueryString(),
+ "/response/numFound==2]",
+ "/response/docs/[0]/id=='99'",
+ "/response/docs/[0]/vector==[1.0, 2.0, 3.0, 4.0]",
+ "/response/docs/[1]/id=='98'",
+ "/response/docs/[1]/vector==[1.0, 2.0, 3.0, 4.0]");
+
+ restTestHarness.delete(ManagedTextToVectorModelStore.REST_END_POINT + "/dummy-1");
Review Comment:
it was cleanup, but not all tests need it, so I added it explicitly, added a line comment to make it clearer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #3151:
URL: https://github.com/apache/solr/pull/3151#issuecomment-2639773032
Added one iteration of polishing, should have addressed @cpoerschke concerns on vectorisation failures (I took inspiration from the lang detect update processor).
I also removed the additional test solr config files addressing @dsmiley concerns.
I'm still puzzled by the testing errors I get (the before/after problems I mentioned in the first comment), any help there would be beneficial.
I think we made progress, I;ll wait some other iterations and then work on the documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1964100880
##########
solr/modules/llm/src/test/org/apache/solr/llm/textvectorisation/update/processor/TextToVectorUpdateProcessorFactoryTest.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.textvectorisation.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.MultiMapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class TextToVectorUpdateProcessorFactoryTest extends TestLlmBase {
+ private TextToVectorUpdateProcessorFactory factoryToTest =
+ new TextToVectorUpdateProcessorFactory();
+ private NamedList<String> args = new NamedList<>();
+
+ @BeforeClass
+ public static void initArgs() throws Exception {
+ setupTest("solrconfig-llm.xml", "schema.xml", false, false);
+ }
+
+ @AfterClass
+ public static void after() throws Exception {
+ afterTest();
+ }
+
+ @Test
+ public void init_fullArgs_shouldInitAllParams() {
+ args.add("inputField", "_text_");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+ factoryToTest.init(args);
+
+ assertEquals("_text_", factoryToTest.getInputField());
+ assertEquals("vector", factoryToTest.getOutputField());
+ assertEquals("model1", factoryToTest.getModelName());
+ }
+
+ @Test
+ public void init_nullInputField_shouldThrowExceptionWithDetailedMessage() {
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Missing required parameter: inputField", e.getMessage());
+ }
+
+ @Test
+ public void init_nullOutputField_shouldThrowExceptionWithDetailedMessage() {
+ args.add("inputField", "_text_");
+ args.add("model", "model1");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Missing required parameter: outputField", e.getMessage());
+ }
+
+ @Test
+ public void init_nullModel_shouldThrowExceptionWithDetailedMessage() {
+ args.add("inputField", "_text_");
+ args.add("outputField", "vector");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Missing required parameter: model", e.getMessage());
+ }
+
+ /* Following tests depends on a real solr schema */
+ @Test
+ public void init_notExistentOutputField_shouldThrowExceptionWithDetailedMessage() throws Exception {
+ args.add("inputField", "_text_");
+ args.add("outputField", "notExistentOutput");
+ args.add("model", "model1");
+
+ Map<String, String[]> params = new HashMap<>();
+ MultiMapSolrParams mmparams = new MultiMapSolrParams(params);
Review Comment:
Thanks! just pushed it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #3151:
URL: https://github.com/apache/solr/pull/3151#issuecomment-2672286942
I've done another round of cleanup, documentation and improvements.
There's still the annoying problem with the beforeClass/afterClass and tests leftovers.
I'll be back in a couple of weeks, feel free to contribute any suggestions!
I believe we are quite close to finalise this contribution, thanks for all the help!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943539435
##########
solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java:
##########
@@ -88,13 +88,33 @@ private static void checkUpdateU(String message, String update, boolean shouldSu
if (response != null) fail(m + "update was not successful: " + response);
} else {
String response = restTestHarness.validateErrorUpdate(update);
- if (response != null) fail(m + "update succeeded, but should have failed: " + response);
+ if (response == null) fail(m + "update succeeded, but should have failed: " + response);
}
} catch (SAXException e) {
throw new RuntimeException("Invalid XML", e);
}
}
+ public static void checkUpdateU(String update, String... tests) {
Review Comment:
At the moment, RestTestBase is common to basically any test using a "REST-based model store"; which the LLM stuff recently added a new variant of and hence RestTestBase is used. RestTestBase is used a lot. Preferrably we wouldn't depend too much on our class hierarchy to accomplish re-usable things. But there's no realistic action to take right now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943637205
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorTest.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TextToVectorUpdateProcessorTest extends TestLlmBase {
+
+ @BeforeClass
+ public static void init() throws Exception {
+ setupTest("solrconfig-llm-indexing.xml", "schema.xml", false, false);
+
+ }
+
+ @Test
+ public void processAdd_inputField_shouldVectoriseInputField()
+ throws Exception {
+ loadModel("dummy-model.json");
+ assertU(adoc("id", "99", "_text_", "Vegeta is the saiyan prince."));
+ assertU(adoc("id", "98", "_text_", "Vegeta is the saiyan prince."));
+ assertU(commit());
+
+ final String solrQuery = "*:*";
+ final SolrQuery query = new SolrQuery();
+ query.setQuery(solrQuery);
+ query.add("fl", "id,vector");
+
+ assertJQ(
+ "/query" + query.toQueryString(),
+ "/response/numFound==2]",
+ "/response/docs/[0]/id=='99'",
+ "/response/docs/[0]/vector==[1.0, 2.0, 3.0, 4.0]",
+ "/response/docs/[1]/id=='98'",
+ "/response/docs/[1]/vector==[1.0, 2.0, 3.0, 4.0]");
+
+ restTestHarness.delete(ManagedTextToVectorModelStore.REST_END_POINT + "/dummy-1");
+ }
+
+ /*
+ This test looks for the 'dummy-1' model, but such model is not loaded, the model store is empty, so the update fails
+ */
+ @Test
+ public void processAdd_modelNotFound_shouldRaiseException() {
+ assertFailedU("This update should fail but actually succeeded", adoc("id", "99", "_text_", "Vegeta is the saiyan prince."));
+
+ checkUpdateU(adoc("id", "99", "_text_", "Vegeta is the saiyan prince."),
+ "/response/lst[@name='error']/str[@name='msg']=\"The model requested 'dummy-1' can't be found in the store: /schema/text-to-vector-model-store\"",
+ "/response/lst[@name='error']/int[@name='code']='400'");
+ }
+
+ @Test
+ public void processAdd_emptyInputField_shouldLogAndIndexWithNoVector() throws Exception {
+ loadModel("dummy-model.json");
+ assertU(adoc("id", "99", "_text_", ""));
+ assertU(adoc("id", "98", "_text_", "Vegeta is the saiyan prince."));
+ assertU(commit());
+
+ final String solrQuery = "*:*";
+ final SolrQuery query = new SolrQuery();
+ query.setQuery(solrQuery);
+ query.add("fl", "id,vector");
+
+ assertJQ(
+ "/query" + query.toQueryString(),
+ "/response/numFound==2]",
+ "/response/docs/[0]/id=='99'",
+ "!/response/docs/[0]/vector==", //no vector field for the document 99
Review Comment:
it took an afternoon almost to find that, it deserved a comment :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943529534
##########
solr/modules/llm/src/test/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessorFactoryTest.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.MultiMapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class TextToVectorUpdateProcessorFactoryTest extends TestLlmBase {
+ private TextToVectorUpdateProcessorFactory factoryToTest =
+ new TextToVectorUpdateProcessorFactory();
+ private NamedList<String> args = new NamedList<>();
+
+ @BeforeClass
+ public static void initArgs() throws Exception {
+ setupTest("solrconfig-llm.xml", "schema.xml", false, false);
+ }
+
+ @AfterClass
+ public static void after() throws Exception {
+ afterTest();
+ }
+
+ @Test
+ public void init_fullArgs_shouldInitFullClassificationParams() {
+ args.add("inputField", "_text_");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+ factoryToTest.init(args);
+
+ assertEquals("_text_", factoryToTest.getInputField());
+ assertEquals("vector", factoryToTest.getOutputField());
+ assertEquals("model1", factoryToTest.getModelName());
+ }
+
+ @Test
+ public void init_nullInputField_shouldThrowExceptionWithDetailedMessage() {
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ SolrException e = assertThrows(SolrException.class, () -> factoryToTest.init(args));
+ assertEquals("Text to Vector UpdateProcessor 'inputField' can not be null", e.getMessage());
+ }
+
+ @Test
+ public void init_notExistentInputField_shouldThrowExceptionWithDetailedMessage() throws Exception {
+ args.add("inputField", "notExistentInput");
+ args.add("outputField", "vector");
+ args.add("model", "model1");
+
+ Map<String, String[]> params = new HashMap<>();
+ MultiMapSolrParams mmparams = new MultiMapSolrParams(params);
+ SolrQueryRequestBase req = new SolrQueryRequestBase(solrClientTestRule.getCoreContainer().getCore("collection1"), (SolrParams) mmparams) {};
Review Comment:
It's an anonymous inner class. What's probably throwing you off is that there are no method overrides, which 99% of the time is the point of doing an anonymous inner class. Here it's because SQRB is abstract so he's forced to subclass it in order to use it. I've been thinking of this case recently and I think we should simply make that impl not abstract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1943553277
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
Review Comment:
I was debugging the flow to have a better understanding of the lifecycle of an update request processor.
From what I see from the test, the factory instantiates a new update request processor every time a new update request is received.
I think it's ok to keep it a class member, but let me see if I can move the instantiation to the factory.
Ideally I wanted that to happen when the factory is initiate but It seems that the update request processor factory is not compatible with resource loading (as far as I debugged and checked)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1964064479
##########
solr/modules/llm/src/java/org/apache/solr/llm/texttovector/update/processor/TextToVectorUpdateProcessor.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.llm.texttovector.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrInputField;
+import org.apache.solr.llm.texttovector.model.SolrTextToVectorModel;
+import org.apache.solr.llm.texttovector.store.rest.ManagedTextToVectorModelStore;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.processor.UpdateRequestProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+
+class TextToVectorUpdateProcessor extends UpdateRequestProcessor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private final String inputField;
+ private final String outputField;
+ private final String model;
+ private SolrTextToVectorModel textToVector;
+ private ManagedTextToVectorModelStore modelStore = null;
+
+ public TextToVectorUpdateProcessor(
+ String inputField,
+ String outputField,
+ String model,
+ SolrQueryRequest req,
+ UpdateRequestProcessor next) {
+ super(next);
+ this.inputField = inputField;
+ this.outputField = outputField;
+ this.model = model;
+ this.modelStore = ManagedTextToVectorModelStore.getManagedModelStore(req.getCore());
+ }
+
+ /**
+ * @param cmd the update command in input containing the Document to process
+ * @throws IOException If there is a low-level I/O error
+ */
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+ this.textToVector = modelStore.getModel(model);
+ if (textToVector == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The model requested '"
+ + model
+ + "' can't be found in the store: "
+ + ManagedTextToVectorModelStore.REST_END_POINT);
+ }
+
+ SolrInputDocument doc = cmd.getSolrInputDocument();
+ SolrInputField inputFieldContent = doc.get(inputField);
+ if (!isNullOrEmpty(inputFieldContent, doc, inputField)) {
+ String textToVectorise = inputFieldContent.getValue().toString();//add null checks and
+ float[] vector = textToVector.vectorise(textToVectorise);
+ List<Float> vectorAsList = new ArrayList<Float>(vector.length);
+ for (float f : vector) {
+ vectorAsList.add(f);
+ }
+ doc.addField(outputField, vectorAsList);
+ }
+ super.processAdd(cmd);
+ }
+
+ protected boolean isNullOrEmpty(SolrInputField inputFieldContent, SolrInputDocument doc, String fieldName) {
Review Comment:
addressed, resolving this, feel free to open a new comment if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1964094479
##########
solr/modules/llm/src/test/org/apache/solr/llm/textvectorisation/update/processor/TextToVectorUpdateProcessorFactoryTest.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.llm.textvectorisation.update.processor;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.MultiMapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.llm.TestLlmBase;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+
+public class TextToVectorUpdateProcessorFactoryTest extends TestLlmBase {
+ private TextToVectorUpdateProcessorFactory factoryToTest =
+ new TextToVectorUpdateProcessorFactory();
+ private NamedList<String> args = new NamedList<>();
Review Comment:
Agreed and pushed the changes!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #3151:
URL: https://github.com/apache/solr/pull/3151#discussion_r1964083376
##########
solr/modules/llm/src/test/org/apache/solr/llm/textvectorisation/search/TextToVectorQParserTest.java:
##########
@@ -29,6 +30,11 @@ public static void init() throws Exception {
loadModel("dummy-model.json");
}
+ @AfterClass
+ public static void cleanup() throws Exception {
+ afterTest();
+ }
Review Comment:
That's the part that is giving me headache and nasty leftovers in tests (as detailed in the initial comment).
The reason for that was to index once before all the tests, do the tests and then do the cleanup at the end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #3151:
URL: https://github.com/apache/solr/pull/3151#issuecomment-2714076350
Ok, no updates, comments or help in the last three weeks, so at the end of the week, I'll proceed fixing the tests and merging. any help with the test clean up is still welcome!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17632: Text to Vector Update Request Processor [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #3151:
URL: https://github.com/apache/solr/pull/3151#issuecomment-2721866606
Merged! Thanks, everybody, for the help!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org