Adapters: Implementation

[pinkwah]

Refer to #380 on how to define parameters. This is a discussion about how to define the rest.

An adapter adapts a CO2 model for use in Acidwatch. The adapter therefore defines some static metadata:

• Model ID (eg. arcs)
• Model name (eg. ARCS for GUI)
• (perhaps other metadata in the future like icon, links, AccessIT link etc)
• A definition of possible parameters and their default values
• A list of supported substances and their default concentrations.

And two functions:

• A function that, given user info, can determine whether the user can access the model. Default is to allow.
• The run function, that takes concentrations and parameters and outputs new concentrations alongside metadata.

Functions should exist that can convert a given adapter to a JSON-able representation so we can display it in the frontend. Bonus points if it's possible to invoke via CLI (acidwatch run arcs NO=2 temperature=10)

[pinkwah]

Adapter A (instanced)

We have a BaseAdapter class, which uses __init_subclass__ method from Python to automatically register it as a known adapter. It can also define an __init__ to do some extra validation. We instantiate this class for each run simulation request we get and then delete it.

class Example(BaseAdapter):
    model_id = "example"
    model_name = "Example"
    concentrations = { "H20": 1, ... }

    async def run(self) -> dict[str, float]:
        # self.concentrations are user-provided in this context
        new_concs = my_example_sim(self.concentrations)
        return new_concs

Depending on how we do the parameters, we can have one of the following:

# Pydantic
class ExampleParameters(BaseParameters):
    some_param: int = Parameter(100, label="Some parameter", description="It does something", unit="?")

class Example(BaseAdapter)
    model_id = "example"
    model_name = "Example"
    concentrations = { "H20": 1, ... }
    parameters: ExampleParameters

    async def run(self) -> dict[str, float]:
        # self.concentrations are user-provided in this context
        # self.parameters are user-provided and validated before calling this method
        new_concs = my_example_sim(
            self.concentrations,
            some_param=self.parameters.some_param  # Note: we can't write self.some_param in this scheme
        )
        return new_concs

# Custom
class Example(BaseAdapter)
    model_id = "example"
    model_name = "Example"
    concentrations = { "H20": 1, ... }
    some_param: int = Parameter(100, label="Some parameter", description="It does something", unit="?")

    async def run(self) -> dict[str, float]:
        # self.concentrations are user-provided in this context
        # self.[parameter] are user-provided and validated before calling this method
        new_concs = my_example_sim(
            self.concentrations,
            some_param=self.some_param
        )
        return new_concs

[pinkwah]

Adapter B (class methods)

We don't want state, so we can make use of @classmethod to be explicit about it.

class Example(BaseAdapter):
    model_id = "example"
    model_name = "Example"
    concentrations = { "H20": 1, ... }

    @classmethod
    async def run(self, concentrations: dict[str, float]) -> dict[str, float]:
        # self.concentrations are user-provided in this context
        new_concs = my_example_sim(concentrations)
        return new_concs

With parameters. (Either parameter scheme fits this style)

# Pydantic
class ExampleParameters(BaseParameters):
    some_param: int = Parameter(100, label="Some parameter", description="It does something", unit="?")

class Example(BaseAdapter)
    model_id = "example"
    model_name = "Example"
    concentrations = { "H20": 1, ... }

    @classmethod
    async def run(self, concentrations: dict[str, float], parameters: ExampleParameters) -> dict[str, float]:
        # self.concentrations are user-provided in this context
        # self.parameters are user-provided and validated before calling this method
        new_concs = my_example_sim(
            concentrations,
            some_param=parameters.some_param
        )
        return new_concs

This scheme requires that the user only use @classmethod. Class methods aren't too common in Python nowadays.

[lars-petter-hauge]

I have argued quite a bit to avoid state in these adapters.

Am I over exaggerating the point of state? I worry that if we start having state in the adapter we could end up in weird circumstances. Say if we somehow end up instantiating the adapter classes before registering them, and use the same adapter instance for each call, with multiple parallell calls..

Point being I would think we could keep things simplified if it is a use and throw experience.

[pinkwah]

I think I'm less concerned because I think it's reasonable to define that the lifetime of an adapter instance is between a run_simulation request and the simulation being completed and state is thrown away. If the model is stateful, this state is very limited.

[lars-petter-hauge]

Alright.. I do think though that if there is no need to keep state, that should be a good enough reason to avoid it. If we see that not having state makes the adapter interface awkward, then we should turn around. If the adapter for instance would use some session context, I would be all on board, cause otherwise this complexity would have had to be handled on the caller side.. But the entire point here is that there is no need to handle things like that.

I'll end with; it's not that important, and hence I won't have a strong opinion on it.

[lars-petter-hauge]

Uhm..

How do we handle:

A function that, given user info, can determine whether the user can access the model. Default is to allow.

Currently it is AcidWatch backend that does this here:

acidwatch/backend/src/acidwatch_api/authentication.py

Lines 105 to 113 in f9d2e22

	def acquire_token_for_downstream_api(api: MODEL_TYPE, jwt_token: str) -> str:
	match api:
	case MODEL_TYPE.CO2SPEC:
	scope = configuration.CO2SPEC_API_SCOPE
	case _:
	raise ValueError(f"Unsupported model type: {api}")
	result = confidential_app.acquire_token_on_behalf_of(
	user_assertion=jwt_token, scopes=[scope]
	)

How are we going to handle the auth part? Expect that all authorization is scope based? And every adapter must provide a scope if they want to be able to authorize user? If that is the case, we don't need a function in the adapter, we only need the adapter to provide scope.

If the adapter is going to handle this itself, is it going to be optionally granted an instance of ConfidentialClientApplication, and a user token and itself check if it is possible to get another token? (dont think that is a good idea..).

Perhaps having each adapter providing the scope makes the most sense (and still be configured through env variables though)

[pinkwah]

Adapter C (decorated functions)

We can use Flask/FastAPI-style decorations to define the run function. That way we can get class-method style without using class methods.

from acidwatch import Adapter, Parameter, BaseParameters

adapter = Adapter("arcs", name="ARCS", concentrations={"H2O": 1, ...})

@adapter.set_run 
async def run(concentrations: dict[str, float]) -> dict[str, float]:
    new_concs = my_example_sim(concentrations)
    return new_concs

or, of course:

from acidwatch import Adapter, Parameter, BaseParameters

async def run(concentrations: dict[str, float]) -> dict[str, float]:
    new_concs = my_example_sim(concentrations)
    return new_concs

adapter = Adapter("arcs", name="ARCS", concentrations={"H2O": 1, ...}, run=run)
# Maybe:
# register_adapter("arcs", name="ARCS", concentrations={"H2O": 1, ...}, run=run)

For parameters we can use the same scheme where it's a separately defined class whose instance is passed to run function.

[lars-petter-hauge]

parts of me think that if we end up adding more content on the adapter, this setup might end up looking a bit messy.. Shouldn't gold plate of course.. And it is certainly possible to revisit this discussion if we want to reconsider how we do things (and start with the simplest way..)

[pinkwah]

Yeah I was thinking the same thing. The pro of this is that a function doesn't have state (ofc excluding global state). The con is that, as you say, it may become unwieldy.

[lars-petter-hauge]

Model ID (eg. arcs)
Model name (eg. ARCS for GUI)
(perhaps other metadata in the future like icon, links, AccessIT link etc)
A definition of possible parameters and their default values
A list of supported substances and their default concentrations.

Slight side note. Perhaps Display name over model name as to be explicit about usage.

We could consider a short (e.g. tooltip) and long description (we really don't have enough info about the models currently, we should support that).

And we would like to include version of the model. We might want to embed this in the metadata though from the adapter, as it might not be available until the run function is executed (as in the response of the web call includes the version). I think it would be fair to require version in some form.

A list of supported substances and their default concentrations.

What would be the reason to expect default concentrations? Could we not have the same default concentrations for all? (and only use the default values for the components that the model accepts, and 0 if we haven't specified one). I think for the user in the frontend it would be beneficial to both keep order and default concentrations identical for all models.

Considering example B, they would be a list of concentrations instead. Perhaps even call them accepted_concentrations (or a better word..)

# Pydantic
class ExampleParameters(BaseParameters):
    some_param: int = Parameter(100, label="Some parameter", description="It does something", unit="?")

class Example(BaseAdapter)
    model_id = "example"
    model_name = "Example"
    accepted_concentrations = ["H2O", ...]

    @classmethod
    async def run(self, concentrations: dict[str, float], parameters: ExampleParameters) -> dict[str, float]:
        # self.concentrations are user-provided in this context
        # self.parameters are user-provided and validated before calling this method
        new_concs = my_example_sim(
            concentrations,
            some_param=parameters.some_param
        )
        return new_concs

I just don't like to have the concentrations specified twice, cause we are never going to use the concentrations defined in the adapter, and hence they kinda lie about their presence.

[pinkwah]

I agree display_name is better and to include model version.

I guess I also agree on the concentrations. But what should the defaults be and where do we specify them in that case? In my mind we could use the default concentrations to run a smoke-test, but we could also generate the concs via hypothesis.

[lars-petter-hauge]

In my mind we could use the default concentrations to run a smoke-test, but we could also generate the concs via hypothesis.

If we start with hypothesis for models in acidwatch aren't kinda going towards testing third party libraries? Each of them should rather use hypothesis. What we want to test is that the adapter implementation is correct. Should suffice with a set of input values?

But what should the defaults be and where do we specify them in that case?

Can ask some users what they think would be interesting defaults. If they have no opinion we use a set from one of the models currently implemented. But where they should be..

Maybe front-end actually, at is it not specific to any model, but rather to how the interface is presented to the user.

[lars-petter-hauge]

Ref #381 (reply in thread)

A thought on the authentication and authorization topic.

I think we can keep things fairly close to what they are today, and use the On-Behalf-Of (OBO) flow. That is, we have authentication in the frontend and authorization in the backend. Frontend allows users to prove who they are, add a token, which is sent to backend with every request. The backend then tries to retrieve an on behalf of token in order to call other services.

In this case the Adapters only need to provide their scope. The backend performs the obo flow and retrieves an access token. The run method in the adapter must then allow for an optional token to be passed with it, if that is necessary, in order to include it whenever it calls downstream apis.

@fastapi_app.get("/run")
def run():
    adapter = ArcsAdapter() # or provide concs and settings here..
    token = acquire_token_for_downstream_api(adapter.scope) if adapter.require_auth else None
    adapter.run(concs, settings, token)


def acquire_token_for_downstream_api(scope: str, jwt_token: str) -> str:
    result = confidential_app.acquire_token_on_behalf_of(
        user_assertion=jwt_token, scopes=[scope]
    )
    if "error" in result:
        logger.error(result["error"])
        raise HTTPException(401, result["error_description"])
    return result["access_token"]  # type: ignore

(we probably would like to perform validation checks on tokens coming from the front end at all times, not sure if that is the case. We should have a Depends on all urls, which, in the case that a token is included, checks that it is valid. But that is a different topic perhaps, and is not specific for adapters..)