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

Clean up/simplify build after MATLAB removal #123

Closed
samcunliffe opened this issue Oct 5, 2022 · 0 comments · Fixed by #139
Closed

Clean up/simplify build after MATLAB removal #123

samcunliffe opened this issue Oct 5, 2022 · 0 comments · Fixed by #139
Labels
housekeeping Code cleanup technical Technical and meta issues, not related to physics but infrastructure. ux Related to the user experience (CLI, workflow etc)
Milestone
v1.1 Removing MAT...

Comments

@samcunliffe
Copy link
Member

samcunliffe commented Oct 5, 2022 •
edited by willGraham01
Loading 

   Worth breaking this out into another issue, also mentioning that we should turn this option from a compile-time option to a command-line option.

Originally posted by @willGraham01 in #122 (comment)

As noted in the developer doc and #122, the current method of switching the solver options is to recompile the executable, passing in the DERIVATIVE_TYPE flag as either FD (finite difference) or PS (pseudo-spectral). This is inconvenient from a user standpoint, as ideally one should be able to specify the desired solver from the command-line interface. However, the MATLAB dependency - specifically the limitation of mexFunction to take $>4$ arguments - does not allow for this.

Once the MATLAB dependency has been removed (#70), we can look to address this.

Dependencies
@samcunliffe samcunliffe mentioned this issue Oct 5, 2022
Merged
@samcunliffe samcunliffe added technical Technical and meta issues, not related to physics but infrastructure. ux Related to the user experience (CLI, workflow etc) housekeeping Code cleanup labels Oct 5, 2022
@samcunliffe samcunliffe mentioned this issue Oct 18, 2022
Closed
samcunliffe added a commit that referenced this issue Oct 19, 2022
@samcunliffe
Compiler flag FDFLAG → command-line option
e6464df 
Removes FDFLAG and replaces it with a command-line option. Revert
changes to build testing and solve #123. No need to mark pytest tests.
@samcunliffe samcunliffe mentioned this issue Oct 19, 2022
Merged
@samcunliffe samcunliffe linked a pull request Oct 19, 2022 that will close this issue
Compiler flag FDFLAG → command-line option. #139
Merged
@samcunliffe samcunliffe mentioned this issue Oct 21, 2022
Merged
samcunliffe added a commit that referenced this issue Oct 25, 2022
@samcunliffe @giordano @willGraham01
Compiler flag FDFLAG → command-line option. (#139)
ac15211 
* Rename mexFunction.

* Compiler flag FDFLAG → command-line option

Removes FDFLAG and replaces it with a command-line option. Revert
changes to build testing and solve #123. No need to mark pytest tests.

* Forgot to commit changes to linux build.

* Apply suggestions from code review

Co-authored-by: Mosè Giordano <[email protected]>

* Sensible name for mexFunction, and DerivativeMethod → SolverMethod.

Co-authored-by: Will Graham <[email protected]>

* Final typos and language fixes.

* Aaaaand remove the pytest marks.

Co-authored-by: Mosè Giordano <[email protected]>
Co-authored-by: Will Graham <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
housekeeping Code cleanup technical Technical and meta issues, not related to physics but infrastructure. ux Related to the user experience (CLI, workflow etc)
Projects
None yet
Milestone
v1.1 Removing MATLAB C++ header depen...
Development

Successfully merging a pull request may close this issue.

Compiler flag FDFLAG → command-line option. UCL/TDMS
1 participant
@samcunliffe