inference using URL #576
inference using URL #576
Conversation
|
|
||
| results : JobResults = training_job.get_results() | ||
| links = results.get_metadata()['links'] | ||
| ml_model_metadata_url = [link for link in links if 'ml_model_metadata.json' in link['href']][0]['href'] |
There was a problem hiding this comment.
Why getting the model URL from the links? Isn't this an asset in the batch job results?
Also, this "ml_model_metadata.json" looks like a VITO-specific implementation detail. Can this be generalized. Or at least documented why you are looking for that string?
There was a problem hiding this comment.
This is within the metadata indeed but different than the Assets's link. The link in the assets is as tar.gz . It throws a JSON error when trying to load it using that specific url and same is with the canonical link.
ref: Open-EO/openeo-geopyspark-driver#562
There was a problem hiding this comment.
If I understand save_ml_model correctly (https://github.com/Open-EO/openeo-processes/pull/441/files#diff-0aae18f348c05c0f26c5caee2dcb0ca4af218a55287c9cbf8a286fc35b8faae5) there should be a asset that is a STAC Item that is to be used with load_ml_model.
that 'ml_model_metadata.json' reference in the current docs is highly implementation specific and should be avoided
There was a problem hiding this comment.
I guess we are missing some specification to allow proper extraction of the model's URL, so I started the discussion at https://github.com/Open-EO/openeo-processes/pull/441/files/b162040dea04acd12fad73166274bbff370ed29b#r1650820880
There was a problem hiding this comment.
We need a better solution for
[link for link in links if 'ml_model_metadata.json' in link['href']][0]['href']
this construct is quite brittle and implementation specific.
Also note that https://github.com/Open-EO/openeo-processes/pull/441/files/b162040dea04acd12fad73166274bbff370ed29b#r1650872242 explicitly says that this the ML Model extension is outdated and going to be replaced, so it is very likely this is going to break
There was a problem hiding this comment.
This is apparently the next-gen ML extension
I think we better make sure we make this snippet future-proof
|
@soxofaan, are additional changes needed ? |
|
|
||
| Next, load the model from the URL:: | ||
|
|
||
| model = connection.load_ml_model(id=ml_model_metadata_url) |
There was a problem hiding this comment.
just do
| model = connection.load_ml_model(id=ml_model_metadata_url) | |
| model = connection.load_ml_model(ml_model_metadata_url) |
I think the id argument is not a very good name and we might change that in the future, so just avoid using it to keep these docs more robust
There was a problem hiding this comment.
| dimension="bands" | ||
| ) | ||
|
|
||
| predicted.download("predicted.GTiff") | ||
|
|
||
|
|
||
| We specified the model here by batch job id (string), | ||
| We specified the model here by URL corresponding to the |
There was a problem hiding this comment.
This is not correct, you specify the model with model, which is a MlModel instance, not a URL
|
|
||
| results : JobResults = training_job.get_results() | ||
| links = results.get_metadata()['links'] | ||
| ml_model_metadata_url = [link for link in links if 'ml_model_metadata.json' in link['href']][0]['href'] |
There was a problem hiding this comment.
We need a better solution for
[link for link in links if 'ml_model_metadata.json' in link['href']][0]['href']
this construct is quite brittle and implementation specific.
Also note that https://github.com/Open-EO/openeo-processes/pull/441/files/b162040dea04acd12fad73166274bbff370ed29b#r1650872242 explicitly says that this the ML Model extension is outdated and going to be replaced, so it is very likely this is going to break
Updated the documentation to use URL based method to load the model