-
Notifications
You must be signed in to change notification settings - Fork 5
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
Updates Examples #262
Updates Examples #262
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, please see comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this file is changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to revisit the possibility of linting and formatting the code. And maybe an issue to address some of the minor linter things such as replacing dlmread
usage with readmatrix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I looked into that. The use of ".layers" as a file format meant I couldn't do the simple switch, so I decided to leave it for now.
properties (TestParameter) | ||
layersFile = {'test0.layers', 'test1.layers', 'test2.layers', 'test3.layers', 'test6.layers', 'test7.layers'} | ||
dataFile = {'test0.dat', 'test1.dat', 'test2.dat', 'test3.dat', 'test6.dat', 'test7.dat'} | ||
% layersFile = {'test0.layers', 'test1.layers', 'test2.layers', 'test3.layers', 'test0.layers', 'test1.layers', 'test6.layers', 'test7.layers'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why the comments are here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to make sure the correspondence between dat files and layers files was noted down in the main test. For the excluded tests they use previous layers files.
The files are direct from here….
https://github.com/reflectivity/analysis/tree/master/validation/test/unpolarised/layers
..so I decided to just keep the ORSO filenames to keep things simple (i.e. if they update, then we can just use….)
A
From: Paul Sharp ***@***.***>
Date: Wednesday, 21 August 2024 at 12:05
To: RascalSoftware/RAT ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [RascalSoftware/RAT] Updates Examples (PR #262)
@DrPaulSharp commented on this pull request.
…________________________________
On examples/domains/customLayers/domainsCustomLayerSheet.mlx<#262 (comment)>:
Yeah, I looked into that. The use of ".layers" as a file format meant I couldn't do the simple switch, so I decided to leave it for now.
—
Reply to this email directly, view it on GitHub<#262 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGMNVXVLRD7PWYP6APFSEADZSRX7TAVCNFSM6AAAAABMZZFHHWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENJQGQ4TCMRXGQ>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Adds ORSO validation to tests, and bayesBenchmark to live script examples. Also refactors live scripts and removes unnecessary duplicate orsoTest files.