sagemaker-python-sdk icon indicating copy to clipboard operation
sagemaker-python-sdk copied to clipboard

Passing Pipeline Variable fails when using f-string

Open OleksiiYeromenko opened this issue 2 years ago • 7 comments

Describe the bug Calling get_pipeline when one of the parameters is using a variable or f-string fails. If replace with hard-coded string - everything fine. This was working before and stopped probably after latest update.

To reproduce

    # script processor definition (image build with cdk)
    script_processor = ScriptProcessor(command=['python3'],
                image_uri='{0}.dkr.ecr.{1}.amazonaws.com/{2}:{3}'.format(account_id, region, ecr_repository_name, "latest"),
                role=role,
                volume_size_in_gb=int(processing_volume_size),
                instance_type=processing_instance_type,
                instance_count=int(processing_instance_count),
                base_job_name=f"{base_job_prefix}/feat-eng-deepar",     ######?
                sagemaker_session=sagemaker_session)

Expected behavior The pipeline definition has to be generated.

Screenshots or logs

TypeError Traceback (most recent call last) in 8 model_package_group_name=model_package_group_name, 9 pipeline_name=pipeline_name, ---> 10 base_job_prefix=base_job_prefix, 11 )

~/test-payment-anomaly-detection-p-byvdz8fkg60h/sagemaker-test-payment-anomaly-detection-p-byvdz8fkg60h-modelbuild/pipelines/testaci/pipeline.py in get_pipeline(region, sagemaker_project_arn, role, default_bucket, model_package_group_name, pipeline_name, base_job_prefix) 354 # script processor definition (image build with cdk) 355 script_processor = ScriptProcessor(command=['python3'], --> 356 image_uri='{0}.dkr.ecr.{1}.amazonaws.com/{2}:{3}'.format(account_id, region, ecr_repository_name, "latest"), 357 role=role, 358 volume_size_in_gb=int(processing_volume_size),

/opt/conda/lib/python3.7/site-packages/sagemaker/workflow/entities.py in str(self) 79 def str(self): 80 """Override built-in String function for PipelineVariable""" ---> 81 raise TypeError("Pipeline variables do not support str operation.") 82 83 def int(self):

TypeError: Pipeline variables do not support str operation.

OleksiiYeromenko avatar May 20 '22 10:05 OleksiiYeromenko

is base_job_prefix a Pipeline parameter? If so, typically string operations are not allowed on them. If you need to concat the pipeline parameters and plain strings, use sagemaker.workflow.functions.Join instead.

navaj0 avatar May 23 '22 20:05 navaj0

image_uri='{0}.dkr.ecr.{1}.amazonaws.com/{2}:{3}'.format(account_id, region, ecr_repository_name, "latest"),

Yes, here also image URI with para - image_uri='{0}.dkr.ecr.{1}.amazonaws.com/{2}:{3}'.format(account_id, region, ecr_repository_name, "latest"),

Ok, will try. Just it was from AWS example and was working few months ago.

OleksiiYeromenko avatar May 23 '22 20:05 OleksiiYeromenko

Hi @OleksiiYeromenko sorry for the inconvenience caused.

Ok, will try. Just it was from AWS example and was working few months ago.

Could you provide us the link of the AWS example? We will need to update and fix the usage there.

Context: We recently updated the sagemaker SDK to explicitly throw exceptions to prevent this kind of usage of PipelineVariable (i.e. str(pipelien_var), f"{pipelien_var}" etc.) The reason is, the PipelineVariable is a placeholder which is parsed/rendered only in execution time, which means it's not able to retrieve its value in the compile time (in sdk).

We are preparing a public document explaining this, which will be released soon.

qidewenwhen avatar May 23 '22 21:05 qidewenwhen

Sorry, can't find it now anymore..

But anyway - When I try to use Join in other Pipeline parameter, I'm getting following error, as it's expecting str, not Join..

For example:

     bucket = ParameterString(
        name="S3Bucket", default_value="anomaly-processing"
    )
    
    defval = Join(['s3://', bucket, "/deepar-inputs/test-DeepAR-pipeline/train/"])
        
    destination_train = ParameterString(
        name="DestinationTrainSet", default_value=defval
    )

Error:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-701d9178fa4c> in <module>
      8     model_package_group_name=model_package_group_name,
      9     pipeline_name=pipeline_name,
---> 10     base_job_prefix=base_job_prefix,
     11 )

~/test-payment-anomaly-detection-p-byvdz8fkg60h/sagemaker-test-payment-anomaly-detection-p-byvdz8fkg60h-modelbuild/pipelines/testaci/pipeline.py in get_pipeline(region, sagemaker_project_arn, role, default_bucket, model_package_group_name, pipeline_name, base_job_prefix)
    300 
    301     destination_train = ParameterString(
--> 302         name="DestinationTrainSet", default_value=defval
    303     )
    304     destination_val = ParameterString(

/opt/conda/lib/python3.7/site-packages/sagemaker/workflow/parameters.py in __new__(cls, *args, **kwargs)
    149     def __new__(cls, *args, **kwargs):  # pylint: disable=unused-argument
    150         """Subclass str"""
--> 151         val = cls._implicit_value("", str, args, kwargs)
    152         return str.__new__(cls, val)
    153 

/opt/conda/lib/python3.7/site-packages/sagemaker/workflow/parameters.py in _implicit_value(cls, value, python_type, args, kwargs)
    119         elif kwargs:
    120             value = kwargs.get("default_value", value)
--> 121         cls._check_default_value_type(value, python_type)
    122 
    123         return value

/opt/conda/lib/python3.7/site-packages/sagemaker/workflow/parameters.py in _check_default_value_type(cls, value, python_type)
    135         """
    136         if value and not isinstance(value, python_type):
--> 137             raise TypeError("The default value specified does not match the Parameter Python type.")
    138 
    139 

TypeError: The default value specified does not match the Parameter Python type.

OleksiiYeromenko avatar May 26 '22 12:05 OleksiiYeromenko

Hi @OleksiiYeromenko, seems you're using nested ParameterString here and sorry we don't support this behavior. Can you try this

    destination_train = ParameterString(
        name="DestinationTrainSet", default_value="s3://anomaly-processing/deepar-inputs/test-DeepAR-pipeline/train/"
    )

Then when starting a new execution, you can update the bucket there:

    execution = pipeline.start(parameters={"DestinationTrainSet": "s3:/another_BUCKET/deepar-inputs/test-DeepAR-pipeline/train/"})

Feel free to let us know if you have any other questions.

qidewenwhen avatar May 27 '22 22:05 qidewenwhen

Hi @OleksiiYeromenko, seems you're using nested ParameterString here and sorry we don't support this behavior. Can you try this

    destination_train = ParameterString(
        name="DestinationTrainSet", default_value="s3://anomaly-processing/deepar-inputs/test-DeepAR-pipeline/train/"
    )

Then when starting a new execution, you can update the bucket there:

    execution = pipeline.start(parameters={"DestinationTrainSet": "s3:/another_BUCKET/deepar-inputs/test-DeepAR-pipeline/train/"})

Feel free to let us know if you have any other questions.

Thank you. Yes, in this way it's ok. But anyway it's not very convenient to work such a way..

Also, for example if I don't want to create outside a lot parameters which could be derived from another one, like
prediction_length, context_length from resample_freq. So I want to have only resample_freq input, other params derive from it. How I can do this properly now?

    resample_freq = ParameterString( name="ResampleFrequency", default_value="30min" )
    
    deepar_prediction_length = str(int(180 / int(re.sub("[^0-9]", "", resample_freq))))
    
    context_length = str(int(10080 / int(re.sub("[^0-9]", "", resample_freq))))

OleksiiYeromenko avatar Jul 04 '22 09:07 OleksiiYeromenko

Hi @OleksiiYeromenko, given that we need to perform complicated calculations on resample_freq to get deepar_prediction_length and context_length, it's not able to define these two derived variables, when generating the pipeline definition. Instead, a proper way to do it is: we can add a LambdaStep, which takes in the resample_freq and outputs the deepar_prediction_length and context_length. The subsequent steps can reference the deepar_prediction_length and context_length from the LambdaStep properties.

qidewenwhen avatar Jul 19 '22 23:07 qidewenwhen

No new responses received. Closing this TT for now. Feel free to reopen if you have any other questions.

qidewenwhen avatar Sep 02 '22 18:09 qidewenwhen

Hi, what i noticed. sometimes it's better when using sagemaker to define the fstring as a variable. It's a strange behaviour ... _path = f"s3://example/"

destination_train = ParameterString( name="DestinationTrainSet", default_value="_path") )

eignerfr avatar Jan 12 '23 13:01 eignerfr