-
Notifications
You must be signed in to change notification settings - Fork 2
latest Julia version + Onnx reader #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Hi Phillip! Thanks for your contribution! Could you revise the CI (continuous integration) file to remove the following lines? The current PR could not pass the continuous integration test because of the file changes.
My orignal plan was to wait until https://github.com/FluxML/ONNX.jl is ready to use because it is native to Flux. But it takes much longer than I expected. What's your future plan to maintain VNNlib.jl? It seems to be a better choice than ONNXNaiveNASflux because we don't really need the NAS part, but only a ONNX parser. If VNNlib.jl will be long term supported, it may be a better choice for MV.jl. |
I deleted the lines you indicated from the Note that most tests will probably fail. For the verifiers to work, one would have to implement the changes outlined in My colleague (Samuel) and me plan to maintain We also have some NN verifiers of our own written in Julia (e.g. VeryDiff, NNPoly) that we want to migrate to When |
I bumped the version in the Installing the package should work in that case. |
It looks like
where Do you think upgrading the Otherwise, we could just not test for the latest prerelease and delete the Let me know what you think is the best option. |
Hi Philipp,
Do you mean the change may break the current pipeline? Is there a particular reason that you didn't directly replace the original verify function? If my understanding is correct, the original onnx handling pipeline will be broken anyway after replacing the ONNXNaiveNASflux with you package. Upgrading the julia-actions/setup-julia version should be safe. To test it, I would suggest setup the github workflow on your forked branch. It's free and should be quite intuitive. Thanks! |
The ONNX parser using
NaiveNASflux
andONNXNaiveNASflux
prevented updating to the latest Julia version.So I deleted those packages and replaced them with an ONNX reader I implemented in my colleagues package
VNNLib.jl
(see github repo here: https://github.com/samysweb/VNNLib.jl/tree/extension).
The new ONNX reader is currently still on the
extension
branch ofVNNLib.jl
, but should be on themain
branch soon.An example showing the necessary changes for running the introductory example in the
Readme.md
using the new ONNX reader fromVNNLib.jl
is implemented indevelop/onnx_parser_adaptations.jl
in my fork ofModelVerification.jl
.The modifications I've already done for the version update and a general description of the steps necessary to move to the new ONNX reader are described in the
changes.md
of my fork.A main difference would be that the
propagate_layer_batch()
method would have to dispatch on the subtype ofVNNLib.OnnxParser.Node
instead of directly dispatching on theFlux
operation.But this is necessary, as many ONNX operations don't directly map to
Flux
(e.g.Gemm
with transposed inputs).How does this align with your plans for the future of
ModelVerification.jl
?Feel free to suggest any requirements, that you need to be met.
All the best,
Philipp