fast-aircraft-design / FAST-OAD

FAST-OAD: An open source framework for rapid Overall Aircraft Design
GNU General Public License v3.0
53 stars 26 forks source link

Some variable_descriptions.txt are not read in custom plugin #378

Closed florentLutz closed 3 years ago

florentLutz commented 3 years ago

Describe the bug In a custom FAST-OAD plugin, variable_descriptions.txt are only read if they are next to a module registered using @RegisterOpenMDAOSystem. I'm not sure if this behaviour is expected.

To Reproduce With files in this archive, the following appears when running the main.py file :

Actual Behaviour

Expected behavior Since there is another variable_descriptions.txt in the src/model folder (which a priori is not read), the expected behaviour would have been :

Expected Behaviour

Error message There are no error messages.

Environment

Additionnal context Same behaviour is seen in FAST-GA, the variable_descriptions.txt in fastga/src/models is not read while the one in fastga/src/models/aerodynamics for exemple is read. If we add this file the problem is solved. If the reviewer wants it, we can create a branch to see the "problem" in a more complex context

christophe-david commented 3 years ago

In your example, the unused file is at same level as the module folder you provide in your configuration file (.\src\model\submodel).

FAST-OAD voluntary ignores whatever is outside the folder you provide, but should take into account a variable_descriptions.txt file at .\src\model\submodel\variable_descriptions.txt whether there is registered module next to it or not (as stated here).

If you get the same behavior by providing .\src\model as module folder, then we actually have a bug. Could you please try that?

florentLutz commented 3 years ago

It indeed worked as you explained, thank you for the explanation and the time provided.

There is however a similar behaviour in FAST-OAD-GA, albeit the module_folder is left empty in the .yml, as written in the Additionnal context section of the issue.

I'll try to write a simple code to replicate the behaviour. In the meantime, a workaround has been found, so there is no urgent need for an answer.

Thanks again for the help.

florentLutz commented 3 years ago

I managed to replicate the issue with this code. It is mostly the same as before except now the modules are declared as a plugin. Consequently, module_folder is not used anymore and the behaviour displayed in the first message of the issue is seen.

From what I've tried, it seems that only the variable_descriptions.txt next to a file containing a @RegisterOpenMDAOSystem are read. This observation prompted the workaround that can be seen in FAST-OAD-GA.

I ran the code with a conda environement with Python 3.8 and did a poetry install in a separate folder on my computer which gave the same behaviour. Do I need to write a separate issue for this problem ?

christophe-david commented 3 years ago

Thanks for taking the time to do this sample code. Very useful.

I hope you took the time to read the doc link I previously provided. In that case, I will greatly appreciate suggestions about how to better formulate the behavior to expect.

The point is that the variable_descriptions.txt that is not read is not in the same folder as a registered component, neither at the root folder of the plugin. If I simply change the plugin declaration to "internal_models" = "variable.model", the file is correctly read. Same happens if I keep the plugin declaration unchanged but if I simply move the file to the variable folder.

BTW I would recommend a more specific plugin name. I would not have expected that having the same plugin name as the internal one from FAST-OAD would work.

florentLutz commented 3 years ago

Thank you for the reply. I confused the root of the plugin and the root of the models folders, so this is my bad.

I deleted the file I added as a work around, modified the pyproject.toml, and now everything works as expected.

Thanks again for your time and sorry for wasting it on a confusion on my part.

christophe-david commented 3 years ago

No problem. Thanks for the feedback.