search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
706 stars 37 forks source link

[REVIEW]: FlexWing-ROM: A matlab framework for data-driven reduced-order modeling of flexible wings #4211

Closed editorialbot closed 1 year ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@urban-fasel<!--end-author-handle-- (Urban Fasel) Repository: https://github.com/urban-fasel/FlexWingROM Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: !--editor-->@Kevin-Mattheus-Moerman<!--end-editor-- Reviewers: @omtazi, @Kevin-Mattheus-Moerman Archive: 10.5281/zenodo.7419465

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/70cc0af28cda2015132999136ce38eee"><img src="https://joss.theoj.org/papers/70cc0af28cda2015132999136ce38eee/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/70cc0af28cda2015132999136ce38eee/status.svg)](https://joss.theoj.org/papers/70cc0af28cda2015132999136ce38eee)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@oriolcg & @omtazi, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

📝 Checklist for @omtazi

📝 Checklist for @Kevin-Mattheus-Moerman

editorialbot commented 2 years ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 2 years ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=1.58 s (365.0 files/s, 92679.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
MATLAB                         520           7153          15352         106601
HTML                            37           1792           3627           9769
Markdown                         7            130              0            445
XSLT                             1             64             25            311
TeX                              1             30              0            277
GLSL                             2             15              7            155
XML                              5             12             28            118
CSS                              1             14              0             57
YAML                             1              0              0             11
-------------------------------------------------------------------------------
SUM:                           575           9210          19039         117744
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Wordcount for paper.md is 1430

editorialbot commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S0962492902000077 is OK
- 10.1146/annurev.fluid.37.061903.175743 is OK
- 10.1016/j.jcp.2017.02.027 is OK
- 10.2514/3.55530 is OK
- 10.1117/12.776137 is OK
- 10.1016/j.paerosci.2012.06.001 is OK
- 10.1017/jfm.2013.163 is OK
- 10.2514/1.J055193 is OK

MISSING DOIs

- 10.1088/1361-665x/aa7c87 may be a valid DOI for title: Aerostructural optimization of a morphing wing for airborne wind energy applications
- 10.2514/1.j059621 may be a valid DOI for title: Concurrent Design and Flight Mission Optimization of Morphing Airborne Wind Energy Wings
- 10.1098/rspa.2020.0079 may be a valid DOI for title: Data-driven nonlinear aeroelastic models of morphing wings for control
- 10.1002/rnc.3586 may be a valid DOI for title: A method to construct reduced-order parameter-varying models
- 10.1016/j.ast.2021.106821 may be a valid DOI for title: The balanced mode decomposition algorithm for data-driven LPV low-order models of aeroservoelastic systems
- 10.1145/800186.810616 may be a valid DOI for title: A two-dimensional interpolation function for irregularly-spaced data
- 10.2514/1.j052715 may be a valid DOI for title: Aero-structural optimization of three-dimensional adaptive wings with embedded smart actuators
- 10.2514/1.j058019 may be a valid DOI for title: Reduced-order dynamic model of a morphing airborne wind energy aircraft
- 10.1242/jeb.204.12.2073 may be a valid DOI for title: Shape, flapping and flexion: wing and fin design for forward flight
- 10.1038/35089071 may be a valid DOI for title: Spanwise flow and the attachment of the leading-edge vortex on insect wings
- 10.1126/science.1142281 may be a valid DOI for title: Bat flight generates complex aerodynamic tracks
- 10.2514/1.36694 may be a valid DOI for title: Aeromechanics of membrane wings with implications for animal flight

INVALID DOIs

- None
editorialbot commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg, @omtazi this is where the review takes place. Here are our review guidelines: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html.

Once you are ready you may post the following comment here (on its own, no other text): @editorialbot generate my checklist

This will create a checklist for you that will guide you through the process. In essence you will test the software and review the short paper and go through the checklist.

Please do not hesitate to contact me if you have any questions.

Thanks again for your help!

omtazi commented 2 years ago

Review checklist for @omtazi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg thanks again for your help with this review. Let me know if you have any questions. Are you able to start the review process too? You can call @editorialbot generate my checklist to get a list like shown :point_up:, this will guide you through the review process. Thanks!

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg :wave: let me know if I can help.

oriolcg commented 2 years ago

Review checklist for @oriolcg

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

oriolcg commented 2 years ago

@Kevin-Mattheus-Moerman, where do I post comments on the review? Directly to the repository?

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg apologies for the slow response. You can comment on small ish things here but if they are larger issues I recommend you open an issue on the project repository on that and then link to it here. Let me know if you have any other questions.

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg, @omtazi thanks again for your help here. Can you provide an update on review progress? Thanks.

omtazi commented 2 years ago

@oriolcg, @omtazi thanks again for your help here. Can you provide an update on review progress? Thanks.

Sorry for the late response, I've been a bit busy lately. I'm hoping to be done by next week on the review

omtazi commented 2 years ago

Hello @Kevin-Mattheus-Moerman @oriolcg , I'm having trouble running the example1 and example2 codes. I get the following message when I run example2_NACA6418_unsteadyFSI.m . Do you have this too ?

`Unable to resolve the name 'parallel.Settings'.

Error in Assembly/adjust_parallel_settings (line 26) ps = parallel.Settings;

Error in Assembly/set.parallelized (line 22) adjust_parallel_settings(self);

Error in Assembly (line 16) self.parallelized = false;

Error in runYetAnotherFEcode (line 147) PlateAssembly = Assembly(myMesh);

Error in generateWingModelStructure (line 1374) [M,K,wP] = runYetAnotherFEcode(wingProperties, wingDesign);

Error in example2_NACA6418_unsteadyFSI (line 46) wingModelStructure = generateWingModelStructure(wingDesign,simParam);`

Kevin-Mattheus-Moerman commented 2 years ago

@urban-fasel can you help @omtazi to address this issue? :point_up:

urban-fasel commented 2 years ago

Hi @Kevin-Mattheus-Moerman, yes! And sorry for not directly replying to the issue.

Hi @omtazi, the issue is related to the "yetAnotherFEcode" that we use to build the FE model of the wing. This package uses the "Parallel Computing Toolbox". If you install the toolbox, you should be able to run the code.

In case other toolboxes are missing, the required toolboxes for e.g. example2 can be found and printed in the command line like this: [~,pList] = matlab.codetools.requiredFilesAndProducts('example2_NACA6418_unsteadyFSI.m'); {pList.Name}'

I hope this helps and please let me know if you still get the error message.

Kevin-Mattheus-Moerman commented 2 years ago

@omtazi can you test this approach :point_up:

Kevin-Mattheus-Moerman commented 2 years ago

@omtazi :wave:

Kevin-Mattheus-Moerman commented 2 years ago

@omtazi :wave:

oriolcg commented 2 years ago

@urban-fasel , I get the following error when executing the code:

  Control System Toolbox
  DSP System Toolbox
  Model Predictive Control Toolbox
  Signal Processing Toolbox

Error in generate_gramian (line 19)
            sys = ss(A_aIODMDc,B_aIODMDc,C_aIODMDc,D_aIODMDc,0.006);

Error in Obtain_BMD_model (line 67)
    [W_o, W_c] = generate_gramian(Snapshot(:,:,k), Snapshot_input(:,:,k), V0, type_string_gram, loadGramians, rankMax, simInput);

Error in ROMsim (line 33)
        ROMs = Obtain_BMD_model(Snapshot, Snapshot_input, V0, type_string, rankROM.rankMaxBMD, simInput);

Error in ROM (line 119)
ROMsim(V0,rankROM,type_string,dT,iTest,simInput,simOutFULL);

Error in MAIN (line 187)
    ROM(paramFSI);

Please, state in the README the required toolbox that should be installed before executing

oriolcg commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

Kevin-Mattheus-Moerman commented 2 years ago

@omtazi @frederickgosselin can you please start the review process? Thanks

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg thanks again for your help here. I see you encountered a "missing toolboxes" type of error. Can you install those and proceed or is this an obstacle?

@urban-fasel please do add toolbox requirements to the README.

urban-fasel commented 2 years ago

@oriolcg and @Kevin-Mattheus-Moerman thanks for the comments, I added the toolbox dependencies to the readme:

Dependencies

The following matlab toolboxes are required to run the code:

omtazi commented 2 years ago

@Kevin-Mattheus-Moerman Hi, sorry for the delay, I'll manage to get it done by the beginning of next week.

omtazi commented 2 years ago

@Kevin-Mattheus-Moerman I checked most of the items of the checklist. However, I can't find the "Community guidelines" part of the documentation for people wishing to contribute to the software.

urban-fasel commented 2 years ago

@omtazi I forgot to add the community guidelines, I just added them to the readme!

omtazi commented 2 years ago

@urban-fasel Thank you. Then @Kevin-Mattheus-Moerman I checked every item on the list, please let me know if you need me to do anything else.

omtazi commented 2 years ago

Hello, I wrote a synthesis of the comments I had on the code and the article. Let me know if you need anything else.

FlexWingROM is a useful Matlab framework that allows to model flexible wings. The paper clearly gives context on the need that the code aims at responding and that motivates this work, and pedagogically explains the approach adopted to contribute to the field of aeroelastic reduce-order modeling. The code is split in different parts that can be used independently: it allows the user to model a flexible wing and solve the corresponding FSI problem using a parameterized wing generator and a finite element solver. Reduced-order models can be built using snapshots of the simulation using three different ROM methods. The code allows the user to compare the results obtained with the ROM to the FE results to evaluate the accuracy of the model; it also allows to compare the performance of the three ROM methods to each other. The documentation is clear and well written. The installation goes smoothly as the necessary toolboxes are stated. The paper and the GitHub instructions are very helpful in understanding the approach of this work and the theory behind the code. The implementation is pretty explicit, and the comments written in the code allow to follow the reasoning of the authors. It seems to me that, given enough time, one can understand each step of the implementation using the comments in the code and the documentation. Examples are given so the user can try by themselves the different features of the framework and play with the main parameters. One possible improvement that comes to my mind would be to explicitly mention in the documentation all the physical quantities that are represented in the output graphs, as their symbol might not always be sufficient to unambiguously identify them. To sum up, FlexWing-ROM is an interesting tool to model flexible wings using ROMs that could prove to be useful for students and researchers and deserves to be published in JOSS.

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg :wave: can you provide an update on this review? Did you manage to add those missing toolboxes?

@oriolcg thanks again for your help here. I see you encountered a "missing toolboxes" type of error. Can you install those and proceed or is this an obstacle?

Kevin-Mattheus-Moerman commented 2 years ago

@oriolcg please can you get back to us on this review? Thanks!

Kevin-Mattheus-Moerman commented 1 year ago

@oriolcg :wave:

Kevin-Mattheus-Moerman commented 1 year ago

@oriolcg can you please resume your review here? Thanks

Kevin-Mattheus-Moerman commented 1 year ago

I have emailed @oriolcg and they indicated they cannot continue the review here. I will now proceed to remove @oriolcg as a reviewer. I will look for an alternative reviewer now or I will review this work myself if needed.

Kevin-Mattheus-Moerman commented 1 year ago

@editorialbot remove @oriolcg as reviewer

editorialbot commented 1 year ago

@oriolcg removed from the reviewers list!

Kevin-Mattheus-Moerman commented 1 year ago

@editorialbot add me as reviewer

editorialbot commented 1 year ago

@Kevin-Mattheus-Moerman added to the reviewers list!

Kevin-Mattheus-Moerman commented 1 year ago

Review checklist for @Kevin-Mattheus-Moerman

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Kevin-Mattheus-Moerman commented 1 year ago

@urban-fasel I have opened some issues: https://github.com/urban-fasel/FlexWingROM/issues/1 and https://github.com/urban-fasel/FlexWingROM/issues/2, with some serious concerns. Please address these at your earliest convenience.

Kevin-Mattheus-Moerman commented 1 year ago

@omtazi were you able to run MAIN.m and unitTest_FlexWingROM.m? Several functions were not included in the source or were included in an incomplete way (see also https://github.com/urban-fasel/FlexWingROM/issues/1).

urban-fasel commented 1 year ago

@Kevin-Mattheus-Moerman, thanks for the comments! I created the code_ext folder, with the complete distmesh code (this should solve https://github.com/urban-fasel/FlexWingROM/issues/1), cosspace, XFOIL, and yetANotherFECode, and added the license files to code_ext. I replaced two mex functions (crossC and dotC) to simplify the code and I also updated the flexWingROM license to GPLv3.

Kevin-Mattheus-Moerman commented 1 year ago

@omtazi were you able to run MAIN.m and unitTest_FlexWingROM.m? Several functions were not included in the source or were included in an incomplete way (see also urban-fasel/FlexWingROM#1).

Actually it may have been a Ubuntu Linux issue and that it worked on Windows.

Kevin-Mattheus-Moerman commented 1 year ago

@urban-fasel I opened some more issues: https://github.com/urban-fasel/FlexWingROM/issues

Note, it looks like I may need to find an alternative reviewer, as it seems currently your software does not support use on Linux (I use Ubuntu). Can you confirm or advise?

urban-fasel commented 1 year ago

@Kevin-Mattheus-Moerman thanks for your helpful comments! As mentioned directly in the issue, XFOIL is only used for precalculating viscous drag, which I now store in tables for different airfoils that can be loaded if the code is run on Linux. The main code does not use XFOIL and the code should therefore be useful also for Linux users.

Kevin-Mattheus-Moerman commented 1 year ago

@urban-fasel

@Kevin-Mattheus-Moerman thanks for your helpful comments! As mentioned directly in the issue, XFOIL is only used for precalculating viscous drag, which I now store in tables for different airfoils that can be loaded if the code is run on Linux. The main code does not use XFOIL and the code should therefore be useful also for Linux users.

Great, it looks like I'll be able to proceed so. I've opened some more minor issues on your repo.

Kevin-Mattheus-Moerman commented 1 year ago

@urban-fasel the README mentions:

The different methods are introduced in the arXiv paper Fasel et al. (2021) -> link will follow.

Is such a l link or publication now available?

urban-fasel commented 1 year ago

@Kevin-Mattheus-Moerman there's no arXiv link available, I will replace the link with the JOSS paper link.