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.

Sign up for GitHub

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

Jump to bottom

Fix crossects dimensions #199

Merged
merged 5 commits into from
Mar 26, 2021
Merged

Fix crossects dimensions #199

merged 5 commits into from
Mar 26, 2021

Conversation

ngadiyar93
Copy link
Member

@ngadiyar93 ngadiyar93 commented Mar 24, 2021 •
edited
Loading

This PR fixes the following cross-sects to properly use dimensions:

  • breadloaf,

  • hollowCylinder

  • hollowRectangle

@ngadiyar93 ngadiyar93 self-assigned this Mar 24, 2021
@ngadiyar93 ngadiyar93 requested a review from elsevers as a code owner March 24, 2021 14:56
@ngadiyar93 ngadiyar93 linked an issue Mar 24, 2021 that may be closed by this pull request
Need to fix MATLAB cross-sections to properly use dimensions #195
Open
13 tasks
@ngadiyar93 ngadiyar93 mentioned this pull request Mar 24, 2021
Open
13 tasks
Bharat-Ramadas
Bharat-Ramadas reviewed Mar 24, 2021
View reviewed changes
model_obj/crosssects/hollowRectangle/CrossSectHollowRect.m Outdated
w=obj.dim_w;
h=obj.dim_h;
t1=obj.dim_t1;
t2=obj.dim_t2;
t3=obj.dim_t3;
t4=obj.dim_t4;
t4=obj.dim_t4;
axis = DimMillimeter([feval(class(w), 0),feval(class(w), 0)]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be using DimMillimeter explicitly here instead of just feval ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Bharat-Ramadas I think feval makes more sense than DimMillimeter.....For example, the user may not always be specifying the dimension as DimMillimeter and may choose DimInch / DimMeter / any other ?

Copy link
Contributor

@Bharat-Ramadas Bharat-Ramadas Mar 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the lack of clarity, what I meant is why can't axis = DimMillimeter([feval(class(w), 0),feval(class(w), 0)]) be just axis = [feval(class(w), 0),feval(class(w), 0)]

Copy link
Member Author

Choose a reason for hiding this comment

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

@Bharat-Ramadas Gotcha! I have fixed this...

Copy link
Contributor

Choose a reason for hiding this comment

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

Better yet, do this: axis = feval(class(w), [0 0])

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, just made the update.

@ngadiyar93 ngadiyar93 merged commit 3b923a4 into develop Mar 26, 2021
@ngadiyar93 ngadiyar93 deleted the fix-crossects-dimensions branch March 26, 2021 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aimthiazz aimthiazz aimthiazz approved these changes

@elsevers elsevers elsevers approved these changes

@ASHAD1 ASHAD1 Awaiting requested review from ASHAD1

@Bharat-Ramadas Bharat-Ramadas Awaiting requested review from Bharat-Ramadas

Assignees

@ngadiyar93 ngadiyar93

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Need to fix MATLAB cross-sections to properly use dimensions
4 participants
@ngadiyar93 @elsevers @aimthiazz @Bharat-Ramadas