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

Excluding files from merger #80

Open
Mathadon opened this issue Jan 4, 2016 · 5 comments
Open

Excluding files from merger #80

Mathadon opened this issue Jan 4, 2016 · 5 comments

Comments

@Mathadon
Copy link
Contributor

Mathadon commented Jan 4, 2016

In IDEAS we have created a 'weatherbus' connector that contains many more signals and parameters than the Annex 60 version. This causes problems (open-ideas/IDEAS#414) when connecting components from A60 such as IDEAS.Fluid.Sources.Outside_CpLowRise.

The cleanest way to deal with this seems to provide the option exclude some files from being overwritten. This way we can develop our own bus connector, without it being overwritten when merging.

An added functionality may be to include the 'original' A60 code in the bus connector, but to comment it, such that it is easy to compare the two code versions when problems would arise after a merge.

Does this seem to be a good solution to our problem?

@mwetter
Copy link
Member

mwetter commented Jan 5, 2016

@Mathadon , @rubenbaetens
We could add a function set_excluded_files to flag files that should not be merged, and then it would be up to the IDEAS developers to set the files you don't want. I however think this is not a clean solution because then what is now the Annex 60 core may diverge from one library to another.
Would it work in your case to extend from the weather bus and add the functionality that IDEAS needs?
I also don't see why IDEAS.Fluid.Sources.Outside_CpLowRise requires a different connector. Do you add functionality that needs additional information?

@rubenbaetens
Copy link

@mwetter @Mathadon There is indeed no need for another connector for IDEAS.Fluid.Sources.Outside_CpLowRise as we do not add any additional functionality. Though we created other models that required additional information which we originally included in our weaBus, thus making the bus no longer compatible with A60 bus.

Is extending from the A60 weabus and adding parameters a solution here ? As you remain attempting to connect two busses with different parameters set (though with a common base).

@Mathadon
Copy link
Contributor Author

Mathadon commented Jan 5, 2016

Thank you for the suggestion. I already tried using the extends clause but this does not work (dymola error message: incompatible connectors).

Our weather bus connector contains several additional Real and Bus variables that do not exist in the A60 version. Still, it would be convenient to use only one bus within IDEAS. I prefer having incompatible connectors between IDEAS-AixLib rather than having incompatible connectors within IDEAS. The solution I proposed seemed to be the cleanest, although I'm open for other suggestions!

@mwetter
Copy link
Member

mwetter commented Jan 5, 2016

What are these additional variables? Are they of general interest to the other libraries as well?

@Mathadon
Copy link
Contributor Author

Mathadon commented Jan 5, 2016

No, they are mostly variables such as light incidence angles and intensity for windows.

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

No branches or pull requests

3 participants