Skip to content
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

Update model loading to be compliant with new versions of tensorflow #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rmarquarlops
Copy link
Collaborator

This pull request includes :

  1. Model loading update in order to be compliant with new versions of tensorflow :
  • new models.py file which stores the model architectures;
  • config.yaml modified accordingly (output bins are now necessary to load the neural models).
  1. It is now possible to give a config_path in order to switch easily between model versions.
    ex: L2-wave-processor --input_path S1_...SAFE --save_directory /savedir --product_id E99 --config_path /localconfig_17km.yaml

  2. Help display speed using L2-wave-processor -h or --help has been increased by managing imports.

sarwaveifrproc/config.yaml Show resolved Hide resolved
Copy link
Member

@agrouaze agrouaze Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model path is optional, it means that, the processor is able to find the resolution of input L1B/L1C and to pick-up an default associated NN model?
What if a user try to predict wave parameters from L1B and NN model that don't have the same resolution (eg a 17.5km² L1B and a 5km² NN model ) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model path is optional because the processor use the elements defined in the localconfig.yaml by default if anything is missing.
Nothing is done to prevent a prediction with a model which was trained on another resolution than a given SAFE.

To prevent that, it would be necessary to store somewhere which IDs (L1B, L1C) correspond to a model version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add a security assert to make sure the resolution of the tiles stored in the input file correspond to the model tile resolution. Do you think it could be done?
If the name of the model file already contains the resolution, it is may be enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We go for a additional informationgiving the version of the L1C product ID used to train the model used. This product-id will be printed at the begining of the script to perform predictions.

sarwaveifrproc/models.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants