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

Output directory in the location specified in settings[SHARPy][log_folder] #130

Merged
merged 23 commits into from
Apr 5, 2021

Conversation

ngoiz
Copy link
Contributor

@ngoiz ngoiz commented Mar 26, 2021

Over time we've ended with all post procs having their individual folder setting to specify their output directory. This has led to several inconsistencies (some would have folder + / case_name/ included in the code and others not) which always brought trouble when writing input files.

Since the use of different output folders for different postprocs is seldom used, the SHARPy output directory is now consolidated in what is provided in the settings['SHARPy']['log_folder'] which will default to ./output. In this directory, a subsequent directory with the case name will be provided such that all output folders (aero, beam....) will be found in settings['SHARPy']['output'] / settings['SHARPy']['case']. The output folder is also now an attribute of the PreSharpy class and can be usually accessed as data.output_folder.

The folder setting for each postproc has been removed. Tests have been updated.

@ngoiz ngoiz requested review from ArturoMS13 and sduess March 26, 2021 08:34
@sduess
Copy link
Collaborator

sduess commented Mar 26, 2021

Well done, Norberto! It is a valuable change for SHARPy.
The only thing I have noticed is a missing import os in one of the post-processors (postproc/beamloads.py). It causes an error.
One other minor suggestion: Let us not forget about our examples cases (e.g. cases\coupled\simple_HALE) and update them as well.

Copy link
Contributor

@ArturoMS13 ArturoMS13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

thank you very much for the effort to standarise this feature.

I have made some commits:

  • I have removed settings['folder'] from some solvers that still had it
  • I have changed self.dir to self.folders in some solvers for standarisation
  • I have removed the definition of the folder variable in tests and cases
  • I have removed old BeamCsvOutput

I didn't date to touch the optimiser so you are welcome to have a look at it. Can you please doublecheck the changes I made to PickleData and SaveParametricCase? I am not sure about them.

Best regards, Arturo.

@sduess sduess requested a review from ArturoMS13 March 29, 2021 11:38
Still requires an empty dictionary to be defined in the config file
@codecov-io
Copy link

Codecov Report

Merging #130 (29b4e2c) into develop (8cf89c3) will decrease coverage by 0.03%.
The diff coverage is 60.82%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #130      +/-   ##
===========================================
- Coverage    62.74%   62.71%   -0.04%     
===========================================
  Files          123      123              
  Lines        21488    21435      -53     
===========================================
- Hits         13483    13442      -41     
+ Misses        8005     7993      -12     
Impacted Files Coverage Δ
sharpy/postproc/pickledata.py 62.50% <0.00%> (+3.31%) ⬆️
sharpy/postproc/saveparametriccase.py 52.00% <0.00%> (+2.00%) ⬆️
sharpy/sharpy_main.py 56.57% <0.00%> (ø)
sharpy/solvers/lindynamicsim.py 28.57% <0.00%> (-1.16%) ⬇️
sharpy/solvers/stepuvlm.py 85.33% <0.00%> (ø)
sharpy/postproc/createsnapshot.py 43.75% <14.28%> (-1.57%) ⬇️
sharpy/solvers/statictrim.py 45.00% <16.66%> (-1.12%) ⬇️
sharpy/postproc/plotflowfield.py 46.03% <20.00%> (ø)
sharpy/postproc/beamloads.py 36.58% <28.57%> (-2.71%) ⬇️
sharpy/postproc/asymptoticstability.py 64.17% <40.00%> (-0.20%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cf89c3...29b4e2c. Read the comment docs.

@ngoiz ngoiz changed the title Consolidated output directory at the location specified in log_folder Output directory in the location specified in settings[SHARPy][log_folder] Mar 29, 2021
@ngoiz ngoiz merged commit 4cab27a into develop Apr 5, 2021
@ngoiz ngoiz deleted the dev_output branch April 5, 2021 07:37
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

Successfully merging this pull request may close these issues.

4 participants