[Bug fix] Mac path fixes and make outputDir public #874
Conversation
…not work on Mac due to file separator.
akeeste
left a comment
There was a problem hiding this comment.
Hi @ahmedmetin
Thank you for these fixes. I don't have an issue making outputDir public to allow users to specify a new output directory.
Your path updates still work for me on Windows. I'm happy with updating these paths to use fullfile() as that is typically very robust across various OS. Do you also have issues running the following examples on Mac? If so, we should update the relevant bemio.m files as well.
- examples/RM3
- examples/BEMIO/Capytaine
- examples/BEMIO/NEMOH
Lastly, does readAQWA fail on Mac without this update? If so we can keep this into the master branch as it is a bug fix.
Adam
source/functions/BEMIO/readAQWA.m
Outdated
| hydro(F).code = 'AQWA'; | ||
| V182 = 0; % Set AqwaVersion flag "Version 18.2 or larger" to 0 | ||
| tmp = strsplit(ah1Filename,{' ','\','.'}); | ||
| tmp = strsplit(ah1Filename,{' ','\', '/', '.'}); |
There was a problem hiding this comment.
Update using MATLAB filesep to be most robust:
tmp = strsplit(ah1Filename,{' ', filesep, '.'});
|
Hello, Yes, all these examples fail on a Mac, because Mac uses / as the file separator, so suggest to replace them either with filesep or fullfile(). readAQWA() does not fail without this change however the intended behaviour that is on Windows is not achieved, i.e. it is not consistent on different platforms. On Mac hydro.file takes the full path to the file without the . in front, while on Windows it is equal only to the filename without the path. This creates and error later when creating a .h5 file, because it does not want to create the .h5 file in a different folder than you are at for some reason (I think it requires the ./ at the beginning). Best regards, |
Additional BEMIO updates
Replaced hardcoded 'output' string with simu.outputDir.
It should be up to the user to clear the command windows and close the figures.
|
@akeeste is this ready for a merge? |
|
Hi @ahmedmetin Sorry for the delay. Thank you for the updates for WEC-Sim on Mac. Once I confirm that these changes work with our applications repo I will merge this PR. Adam |
* Update README.md adding dev and master build status to README * adding v5.0 release notes * Fix typo in docs. (#898) * Update documentation tutorials to fix OSWEC inertia (#894) * Update tutorials to fix OSWEC inertia * Update terminology.rst * Update tutorials.rst changed OSWEC Iyy and added note Co-authored-by: Kelley Ruehl <kmruehl@sandia.gov> * CI: Split docs jobs | Add color to docs logs | Cancel runs on new push | Add 2021b to MATLAB versions (#862) * Split docs CI into test and build jobs This PR splits the docs CI workflow into two independent jobs. The first "test" job checks the current branch with any warnings triggering a failure. The second jobs builds and deploys the production docs, allowing any warnings to pass. This allows the commit author to see any new issues they may be adding to the docs while not stopping the docs being published. * Try to get color output in log * Remove redundant steps for branch test * Fix duplicate targets using anonymous references * Cancel previous runs if new commits are made * Fix spelling mistake to test concurrency * Limit concurrency to pull requests for the unit tests This is to ensure all commits on the master and dev branches are tested, which will be important if we are going to measure coverage using an external service like codecov. * Fix another spelling mistake * Add R2021b to explicit MATLAB versions tested * resolve code struc table bug * [Bug fix] Mac path fixes and make outputDir public (#874) * Moved simulationClass.outputDir to public. * Replaced paths with fullfile paths in CompareBEMIO.m. The paths didd not work on Mac due to file separator. * Update line 38 in reaadAQWA.m so it works on a Mac. * change filesep in BEMIO examples, update readAQWA fileparts * Update stopWecSim.m Replaced hardcoded 'output' string with simu.outputDir. * Removed clc and close all from initializeWecSim. It should be up to the user to clear the command windows and close the figures. Co-authored-by: Lermart96 <anders.brandt@oceanharvesting.com> Co-authored-by: akeeste <akeeste@sandia.gov> * resolving doc language bug * wecSimPCT Fix (Master) (#870) * Update stopWecSim.m * Update initializeWecSim.m * Update wecSimPCT.m * Fix image bug in PTO-Sim in Library Browser (#896) Co-authored-by: Kelley Ruehl <kmruehl@sandia.gov> Co-authored-by: Matthieu Ancellin <31126826+mancellin@users.noreply.github.com> Co-authored-by: jtgrasb <87095491+jtgrasb@users.noreply.github.com> Co-authored-by: Mathew Topper <damm_horse@yahoo.co.uk> Co-authored-by: Ahmed Rashid <32479811+ahmedmetin@users.noreply.github.com> Co-authored-by: Lermart96 <anders.brandt@oceanharvesting.com> Co-authored-by: yuyihsiang <yyu@nycu.edu.tw> Co-authored-by: Jorge Leon <72461917+jleonqu@users.noreply.github.com>
Small suggestion to have outputDir public instead of private and Fixed paths in CompareBEMIO.m so they work both on Windows and Mac.