Skip to content

Conversation

USERSATOSHI
Copy link
Contributor

Description

What is the purpose of this pull request?

This pull request:

  • adds C implementation for @stdlib/math/base/special/erfcx

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@stdlib-bot stdlib-bot added the Math Issue or pull request specific to math functionality. label Mar 25, 2024
@USERSATOSHI
Copy link
Contributor Author

USERSATOSHI commented Mar 25, 2024

const fs = require('fs');

const convertJstoC = () => {
	const string = fs.readFileSync("./erfcx_y100.js", "utf8");
	const regex = /function/g;
	const arg = /\(\s*([^)]+?)\s*\)/g; // regex to get args inside () 

	const functionSplits = string.split("*/\n"); // split string using */ of the comment 

	const replaced = functionSplits.map((func) => {
		const args = func.match(arg);
		func = func.replaceAll("* @param {number} t -","* @param t  ").replaceAll(" {number}","").replaceAll("* @private\n","").replaceAll("@returns ", "@return    ").replaceAll(regex,"static double"); // convert all jsDoc types to C-Deoxygen as well as convert functions to statis double
		if(args) {
			func = func.replaceAll(args[0], "( const double t )"); // convert arg into const double t ( we used arg[0] as arg[1] is mathematical part of p functions as they also match the regex
		}
		return func;  // return updated 
	});

	console.log(replaced)  // for debug purpose

	return replaced.join("*/\n"); // join replaced string to get full string 
}

const news = convertJstoC();

fs.writeFileSync('erfcx_chebyshev.c', news); // save it to another file from where we can copy it to main codebase

created this script to convert all p0 to p100 functions from JavaScript to C as well as update doc to follow C-Deoxygen format
this code still had JavaScript elements such as var , "use-strict" etc as this script was purely to convert functions from JavaScript to C

and rest of the code was written manually

I have not added this script on pr as this didnt seem important hence added it in comment as an insight

screenshot for output

image

edit: Updated Script to match stdlib conventions as well as updated screenshot output to show visual output

@USERSATOSHI
Copy link
Contributor Author

USERSATOSHI commented Mar 25, 2024

updated all missed @\returns to @\return

now this pr is ready for review

@kgryte kgryte added Feature Issue or pull request for adding a new feature. C Issue involves or relates to C. labels Mar 25, 2024
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
@USERSATOSHI
Copy link
Contributor Author

@kgryte updated all the suggested changes, as well as updated script in the comment and now it should follow the convention:

also updated screenshot for a visual output

Copy link
Member

@Pranavchiku Pranavchiku left a comment

Choose a reason for hiding this comment

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

left a few comments, thanks for working on this!

@kgryte
Copy link
Member

kgryte commented Aug 23, 2024

@gunjjoshi You should now have permissions to take over this PR and directly edit/commit. LMK if not.

@gunjjoshi
Copy link
Member

Made some changes, should be good to go!

Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

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

Didn't notice anything off during code review.

Thanks to @USERSATOSHI for creating the PR and @gunjjoshi for getting it over the finish line!

Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@Planeshifter Planeshifter added Ready To Merge A pull request which is ready to be merged. and removed Needs Changes Pull request which needs changes before being merged. labels Sep 2, 2024
@Planeshifter Planeshifter merged commit 67d2064 into stdlib-js:develop Sep 2, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Issue involves or relates to C. Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. Native Addons Issue involves or relates to Node.js native add-ons. Ready To Merge A pull request which is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: add C implementation for @stdlib/math/base/special/erfcx
6 participants