[tmva][sofie] Fix broken links in the SOFIE README - #23106
Open
Pranav-error wants to merge 2 commits into
Open
Conversation
Three links in tmva/sofie/README.md point at files that no longer exist: - tmva/pymva/src/RModelParser_Keras.cxx - tmva/pymva/src/RModelParser_PyTorch.cxx - tutorials/machine_learning/TMVA_SOFIE_Keras.C The Keras and PyTorch parsers are now implemented in Python under bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tmva/_sofie/_parser/, and the Keras tutorial was ported from .C to .py. Point the links at their current locations. The other nine repository links in the file were checked and resolve correctly.
Contributor
|
Thanks for the PR! Please avoid mentioning the PyTorch and Keras parses alltogether. They will go away very soon: |
Following review feedback: rather than repointing the links, remove the mentions altogether, since both parsers are being removed in root-project#22734 in favour of going through ONNX export. - the parser-generator paragraph now names only the ONNX parser - the TMVA_SOFIE_Keras, TMVA_SOFIE_PyTorch and TMVA_SOFIE_PyTorch_HiggsModel tutorial links are dropped
Author
|
Thanks — that makes sense, and #22734 explains it better than repointing the links would have. Updated the PR accordingly: instead of pointing the Keras and PyTorch links at their current locations, the mentions are removed.
There are no Keras or PyTorch references left in Happy to close this instead if you would rather the README change land as part of #22734 — the removal there will touch the same lines. |
Test Results 23 files 23 suites 3d 16h 1m 57s ⏱️ For more details on these failures, see this check. Results for commit fe86197. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three links in
tmva/sofie/README.mdpoint at files that no longer exist in the repository:tmva/pymva/src/RModelParser_Keras.cxxbindings/pyroot/pythonizations/python/ROOT/_pythonization/_tmva/_sofie/_parser/_keras/parser.pytmva/pymva/src/RModelParser_PyTorch.cxx.../_sofie/_parser/_pytorch/parser.pytutorials/machine_learning/TMVA_SOFIE_Keras.Ctutorials/machine_learning/TMVA_SOFIE_Keras.pyThe Keras and PyTorch parsers were reimplemented in Python and moved under the PyROOT pythonizations;
tmva/pymva/src/now contains only theMethodPy*sources. The Keras tutorial was ported from.Cto.py.I checked every repository link in the file — the other nine (
tmva/sofie_parsers, the ONNX/PyTorch/RDataFrame/RSofieReader tutorials, etc.) all resolve correctly, so this only touches the three broken ones.