Developer's call, 10.05.2021

CADET Development
1

Set default adsorption model to ‘NONE’

@j.hassan made some progress on the PR and simplified the code.

Next steps:

Change SOLUTION_INLET_XYZ and SOLUTION_OUTLET_XYZ depending on flow direction

Since this also turned out to be a bug, we want to prioritize this issue.

In libcadet/model/GeneralRateModel.hpp line 461, the outlet concentrations are determined from the last axial cell of the column. However, we should only return the last axial cell if the velocity is greater than zero and the first axial cell if the velocity is lower than zero.

Open questions

  • How to treat velocity == 0
    => Proposal: Only change cell if sign of velocity changes
  • How to treat ports in 2D-GRM, if some of the ports are positive and others negative

Return concentration of inlet stream instead of concentration in first axial cell

The inlet concentration is determined from the first axial cell in libcadet/model/GeneralRateModel.hpp line 455. Instead, to return the concentration of the inlet stream, we should use a similar approach to GeneralRateModel-LinearSolver.cpp, line 182.

Publication on conda-forge, and vcpkg

On conda-forge, the outdated version of sundials is not provided for windows. Hence, we will first make a minor release to upgrade CADET to use the latest Sundials before proceeding.

In the meantime, @w.heymann will check what is required to also add support for vcpkg.

Save last state of each unit separately

Progress can be checked here.

First, we registered a new bool in the ISolutionRecorder and ensured that the value is read from the configuration file.

Next: Changes need to be made in CADET/include/common/SolutionRecorderImpl.hpp; Probably have to read from the model to get the current state for a unit operation using _models and _dofOffset

Default values/ParameterProvider

At this point, we also had another discussion about where to set default values.
In this particular case, the default values are set directly in the ParameterProvider. In contrast, for other cases (see Set default adsorption model to ‘NONE’), they are only added much later in the process (i.e. the implementation of the unit operation model).

It might make sense to extend the functionality of the ParamamterProvider to provide default values for all parameters. This way, there is only one central point for configuration. It does require some effort but should make configuration more robust. Maybe this might also be a good opportunity when we overhaul the API since we need to provide some functions anyway.

Moreover, we should provide more reasonable default values for other parameters where we can. @w.heymann will compile a list of parameters.

This could be a good idea as a student project or for everyone who wants to get to better know the internal structure of CADET.

Git workflow

After some introduction to branches, forks, and pull requests,

Issue

Description of problem, or feature request with corresponding requirements.

PR

Implementation of a solution to an issue. There can be multiple PRs to solve/close an issue, using different approaches. However, generally only one is accepted.

Next: @j.rao will check whether Pull Requests can also be made from feature branches of the CADET repository. This would also be the preferred workflow.
Edit: can confirm that this indeed works!

2

What parameter’s default values are set in the ParameterProvider and where?

The IParameterProvider interface is only for reading parameters / data. Default values can also depend on the particular unit operation, adsorption model etc. (i.e., there is no “one size fits all”).

The only way I can think of right now is to add functions like

virtual double getDouble(const std::string& name, double defaultValue);

which return the provided default value if the parameter is not set.

3

Sorry, I confused some things here. You are right, they are not set in the ParameterProvider but in the driver.hpp:

If I understand this correctly, in this section some values are set to default values if they do not exist in the ParameterProvider, as they are for the adsorption model in the GRM.cpp and so on.

Now the idea was to move this kind of checking and providing default values to the ParameterProvider (or some other configuration class) s.t. all configuration (even the one that depends on some multiplex or other parameters) is handled at one central point. However, this might be too complicated or not flexible enough. It would make some things easier though, right?

Edit: Thinking about it a bit more, it is probably not a good idea…