-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Refactor mathjs to a plugin-based library #71
Comments
👍 Would love to use the evaluator but don't want to load everything else that I won't use. |
Realise that this is an old thread but this would be feasible if mathjs were to be composed from ES Modules instead of using the // old
const math = require('mathjs');
console.log(math.eval('2 + 2'));
// new
import { evaluate } from 'mathjs'; // eval is a reserved word!
console.log(evaluate('2 + 2')); |
Thanks for your input. ES modules would definitely help, with that you could easily load only specific functions instead of the whole library. There is another dimension though that we cannot easily solve this way: being able to include/exclude specific data types: maybe you're not interested in using Matrices or BigNumbers. Each math.js function contains an implementation for each of the data types, and it's not so trivial to split this up. Note that since this topic was opened, math.js is already changed into a much more modular library, and there are already ways to just include the features that you need: http://mathjs.org/docs/custom_bundling.html I would love to bring this modularity a step further though. |
Yeah, I guess this also depends on how stateful you want a modular version of mathjs. Currently the module looks very stateful; const types = {
// this could be from an es module
'Array, any': function (x, y) {
// use matrix implementation
return algorithm14(matrix(x), y, subtract, false).valueOf();
},
};
function subtractFactory (types) {
return function subtract (a, b) {
return typed('subtract', types);
}
}
export default subtractFactory; Not sure if this is as straightforward as that, but it's just an idea. |
I don't yet understand your idea, but I'm totally open for good ideas here though: the current solution with factory functions works but I find it too complicated, and it doesn't allow you to do tree shaking on specific types. |
Right, so let's try and take a simple transformation, the square of a number. The current implementation means that tree-shaking cannot be done because the square transformation has to account for all of the different permutations of types. However, my idea was to be able to supply the square function with an object of types. In this implementation, instead of injecting the import typed from '../typed';
export function squareNumber (x) {
return x * x;
}
// rest of the methods
export function square (config) {
return typed('square', config);
}
export const squareConfig = {
'number': squareNumber,
// rest of the methods
}; Now, a consumer of this module can supply their own import {square, squareNumber} from 'mathjs';
const squared = square({'number': squareNumber})(2); // 4 They can also supply the defaults to get the full list, now all methods would be bundled. import {square, squareConfig} from 'mathjs';
const squared = square(squareConfig)(2); // 4 I think then you would also be able to provide a wrapped method which would do the above and this would be removed from the tree if it were unused; import {defaultSquare} from 'mathjs';
const squared = defaultSquare(2); // 4 |
That's an interesting idea, thanks! That could work. So instead of exporting a single function from a file, each file should export a function for every signature, and we have to to compose them later on. There is some difficulty in that the functions must be created via a factory function first, this factory function passes configuration and some more things. So every function signature would have to be wrapped in it's own factory function which is then exported. That will give a lot of overhead. I think this is worth an experiment, see where we end up and what difficulties we encounter. (btw sorry for the late reply) |
It has been a while but it's getting more and more important to make mathjs fully modular. I want to propose a change in the core architecture of mathjs, based on the suggestion of @ben-eb. I've also implemented a proof of concept in the branch There are two main issues why we want to change the architecture:
I have been thinking about how to change the archicture such that we can make mathjs fully modular. Sort of like a step from the monolith library
Dependency injectionThe current factory functions looks like: function factory (type, config, load, typed) {
const multiply = load(require('./multiply'))
// ...
return // ... implementation of pow, normally a typed-function
}
exports.name = 'pow'
exports.factory = factory This factory function can only easily be used by My idea for a new dependency injection architecture is: export function createPow(math) {
assertDependencies(this.dependencies, math)
return // ... implementation of pow, normally a typed-function
}
createPow.dependencies = ['multiply', ...] Constructing such a function is more straight forward, it doesn't require a special const math = {}
math.multiply = createMultiply(math)
math.pow = createPow(math)
// ... now you can use math.pow This way, you can load your own version of A drawback is that calling Importing and composing your own functions may look like: import { create } from 'mathjs'
import * as allNumberFns from 'mathjs/number'
import * as arithmeticBignumberFns from 'mathjs/bignumber/arithmetic'
import add as fractionAdd from 'mathjs/fraction/add'
// the imports are usable as is, though they are no typed functions
// importing them into math using `import`
const math = create()
math.import([allNumberFns, arithmeticBignumberFns, fractionAdd])
// now you have functions like `add` supporting numbers and bignumbers Explicit (low level) vs generic (high level) functionsThere are two types of functions:
It will be nice if we can expose these low level functions so they can be used on their own, or can be used to mix and match high level functions. There is one special case here: low level functions may require config, which needs to be passed via dependency injection. A solution is to pass function equal(a, b, config) {
const epsilon = config ? config.epsilon : DEFAULT_EPSILON
return // ...
} For all functions this is a suitable solution, only functions like Low level typed functionsExplicit low level functions, should also have input types defined. This could be done by creating them as a import * as typed from 'typed-function'
export add = typed('add', {
'number, number': function (a, b) {
return a + b
}
}) but this introduces an abstraction layer again (it's not low level anymore), and it gives quite some overhead in the code. Nicer is to attach type information as a property on the function. This way the function can both be used as is, or can be turned into a export function add(a, b) {
return a + b
}
add.signature = 'number, number' Usage: // plain JS usage, no types
const c = add(a, b)
// turn into typed function:
const addTyped = typed(add)
const d = addTyped(a, b) High level functions without dependenciesMore than one third of the high level functions do not need dependencies, and are simply an aggregation of a number of low level functions into a Enclosing them in a factory function is unnecessary overhead. I think though we should keep them as such because of:
With the new modular structure, people simply can compose functions themselves, so if needed there already is a way to work around this overhead and not have it end up in your bundle. File structureThe file structure will have a clear distinction between explicit (low level) and generic (high level) functions. It can look like:
If necessary, we can still put all explit, low level functions in a separate file, but they are probably very small and will only cause unnecessary overhead. For easy consumption of individual functions, we could create a build script which creates shortcuts to individual functions into the root of the project, similar to // high level (only efficient when the bundler supports tree shaking)
import { create, add, multiply } from 'mathjs'
const math = create()
math.import(add, multiply)
math.multiply(2, 3) // 6
// high level
import create from 'mathjs/create'
import multiply from 'mathjs/multiply'
const math = create()
math.import(multiply)
math.multiply(2, 3) // 6
// low level
import { multiply } from 'mathjs/number/multiply'
multiply(2, 3) // 6 Since high level functions need a to be created via factory functions (they need Important here is that you don't have to remember in which group a function is located (like Stay backward compatibleI think the refactoring will take quite some effort, and we need to keep the old solution working whilst migrating piece-by-piece to the new architecture. We must to keep the old type of factory functions working in ExperimentI've worked out an experiment in the branch Demonstrating how it can be used: Some explicit (low-level) functions: A refactored generic (high-level) function FeedbackI'm really exited about finally addressing this issue, and this first proof of concept looks really promising. Please let me know what you think! EDIT 2018-09-15: Updated section "File structure", high level functions cannot directly be imported via ES6, they have to be imported/created too since they are factory functions. |
I like this proposal a lot. 👍 In this part: export function createPow(math) {
assertDependencies(this.dependencies, math)
return // ... implementation of pow, normally a typed-function
}
createPow.dependencies = ['multiply', ...] I worry that is it too easy for someone to miss dependencies. I could see my self doing something like: export function createPow(math) {
assertDependencies(this.dependencies, math)
return function (base, exponent) {
return math.multiply(math.add(base, 5), exponent)
}
}
createPow.dependencies = ['multiply'] Above, export function createPow(math) {
const { multiply, add } = getDependencies(this.dependencies, math)
return function (base, exponent) {
return multiply(add(base, 5), exponent)
}
}
createPow.dependencies = ['multiply', 'add'] I envision the Also I found a typo:
Exlicit -> Explicit |
Thanks for your feedback @harrysarson !
Yes I think this will definitely happen and we should should catch this with unit tests or when loading the function. Your suggestion for an I was thinking in a different direction here: in the unit tests we could create a util function to pass only the listed dependencies to The purpose of the There is an other important topic around using I've fixed the "Exlicit" typo, thanks. |
At the risk of adding to the noise here, the only thing stopping me from using mathjs at the moment is also the fact that it ends up including way more code than I need (so I just end up copying out the parts I want). Maybe instead of a having a function that gets the dependencies - which would add a runtime overhead (which isn't the best option in my opinion), why not just let the tree shakers take care of this sort of thing? You could just switch to using es6 imports in all of your files, and all of the dependencies would need to be included to pass the unit tests, then if a user wanted to actually use a particular function, they could just do: // User script
import { simplify } from 'mathjs/algebra' Inside algebra, you'd include all of the dependencies required to run it, so: // Algebra.js
import { dependencyA } from 'somewhere'
import { dependencyB } from 'another'
// ...
export function simplify () {
} So your tree shaker could just go to work killing off anything it doesn't need. Of course, this would mean using a new file to just export a bunch of stuff, sort of like the one d3 uses; but this would make it so much easier for people to rip out only a small part of the library. |
Thanks for your input @saivan . Making mathjs tree-shake-friendly is exactly what we aim for. It's not as easy as for example the functions of lodash. The difficulty is that most of the mathjs functions use a global configuration, which means they have to be created via a factory function. So we will come quite close to your proposal, except you have to load the imported functions into mathjs after importing, something like this (a copy of an example from my proposal): // high level (only efficient when the bundler supports tree shaking)
import { create, add, multiply } from 'mathjs'
const math = create()
math.import(add, multiply)
math.multiply(2, 3) // 6 If you know a different solution to the config problem I would love to hear it. I have been thinking about passing config as a last, optional argument. I don't see this work nicely in practice from an end-user point of view though. |
Well, maybe hiding this from the user would be possible too. By having a global // math.js
// Make the create class
class create () {
define ( name, fn ) {
}
get ( name ) {
}
}
// Then make an instance
export const math = create() You could then pull it in, inside another file like the add file and define the // add.js
// Get the math factory from the other file
import { math } from "math.js"
// Define the function you want to add
math.define('add', () => {
})
// Export the function for the user to use
export const add = math.get('add') Then the user could just do: // User code
import { add } from "mathjs/add" This would automatically create the factory function once, and then it would This syntax is not polished at all, but just an idea. |
@saivan yes you are right. Thinking about it, this is basically how mathjs works right now already: when doing At least, we could indeed do it like this and it would work with tree shaking and a global config out of the box, with the high-level functions containing support for all datatypes. I don't have a clear picture how we could combine this in a consistent and easy to use way with these other goals of custom configuration and support for just a few data types instead of all. The charming thing of the solution where you're forced to create a mathjs instance yourself is that though a bit verbose, it's really simple to understand and it's simple to mix and match everything you want, and you don't have any global config that can be changed by other components. It's similar to having to create an express js server after you've imported it, or similar to how you create a custom fontawesome bundle. We could make it a little bit less verbose like: import { create, add, multiply } from 'mathjs'
const math = create(create, add)
math.multiply(2, 3) // 6 and we can create "shortcuts" for all kind of combinations of functions: import * as all from 'mathjs/all'
import * as arithmetic from 'mathjs/arithmetic'
import * as pow from 'mathjs/arithmetic/pow'
import * as numberArithmetic from 'mathjs/number/arithmetic'
// ...
const math = create(...) It may be interesting to have something like Or, thinking aloud, have |
I've been thinking about it and I think it will be quite easy to enable user code like The index.js file that is loaded when doing // import factory functions
import createAdd from 'mathjs/arithmetic/add'
import createMultiply from 'mathjs/arithmetic/multiply'
import createPow from 'mathjs/arithmetic/pow'
// create instances
export const add = createAdd({ ... })
export const multiply = createMultiply({ ... })
export const pow = createMultiply({ add, multiply, ... }) Still thinking about how creating a new math instance (the current |
Sorry I didn't respond to your last message. I totally missed it in my feed. I actually like this proposal a lot. But how about also having the // file: mathjs/arithmetic/add
export function createAdd( ...params ) {
}
export function add = createAdd ( ...sensibleDefaults ) This would be really flexible for folks who don't necessarily need to craft their own version of this. I'm just thinking that if I were to import a slew of functions, I wouldn't want to initialise them all. That could potentially get hairy for the end user. If they don't import add, I'm not sure whether the tree-shakers would realise that they don't need to include that code, but I feel like they would. I can't be sure though. |
Ah, that's an interesting idea @saivan, I will keep that in mind! What I was thinking about right now is creating the function instances in an "index" file, separate from the factory functions. I want to create a couple of index files which are usable right away, created with "sensible defaults", like: import { add } from 'mathjs'
add(2,3) // supports any data type import { add } from 'mathjs/number'
add(2,3) // supports only numbers,
// so very small (no BigNumber, Complex, Unit Matrices, etc.) import { add } from 'mathjs/bignumber'
add(2,3) // supports only BigNumbers If possible I would like to have flat index files, so you don't have to remember/know that Let me see how it works out. I plan working on the migration in two weeks from now, really looking forward to work on it. Will keep you posted and probably ask for feedback along the road if you don't mind. |
Happy to take a look of course! You could also avoid flat files by just having another file whose only purpose is to import and export parts. For example, you could do: /// math.js
export const matrix = Object.assign({},
import("...somepath/inverse"),
import("...somepath/eigenvalues"),
...
)
export const arithmetic = Object.assign({},
import("...somepath/add"),
import("...somepath/subtract"),
...
) Then if the user imported
But then if they just want particular files, they could use this import file as a guide to make their own smaller version of the library :) |
Thanks. There are indeed quite some options for importing/exporting. I have to see and experiment a bit what works out best in the case of mathjs. But we will definitely end up with some index files just piping imports/exports I think. |
Perfect 👍 |
I've implemented snapshot testing to validate that both the instances, expression parser, and the ES6 exports contain all functions and constants they should have. Fixed the things that where still missing in the ES6 export :). EDIT: so this basically means that the refactored code has everything working that the previous version had. Next steps is to get the tree-shaking working property, and work out number-only implementations of all relevant functions. |
Fixed tree-shaking not working, see 97c16e7 🎉 , starting to get the hang of it. It's unfortunate but because of the factory functions you have to make clear to Webpack that these function are pure using this |
I've flattened everything in the mathjs instances, and created fallbacks for compatibility. So for example Reason for this is that we need to keep the API's consistent and simple, and the ES6 api only allows flat imports (i.e. |
@josdejong if you want to keep the old API: Import { expression } from ‘mathjs’
Import { SymbolNode } from ‘mathjs/expression’
Import { Complex } from ‘mathjs/type’ Though perhaps flat imports is simpler. Really looking forward to modular mathjs! |
@ChristopherChudzicki Yes that's true, I've been thinking about that option as well. What makes it complicated is that we also want to offer different versions of the API: the full version, or implementations for just numbers or BigNumbers, and somehow want to expose the factory functions. I was thinking about something like this: import { add, multiply } from 'mathjs'
import { add, multiply } from 'mathjs/number' // functions only supporting numbers
import { add, multiply } from 'mathjs/bignumber' // functions only supporting BigNumbers
import { createAdd, createMultiply } from 'mathjs/factory' // factory functions to do some cooking yourself But if we also would have things like |
@josdejong Ah, that would be overwhelming. Makes sense! |
I've worked out the use cases in more detail, and tried to come up with a simple API to support all the different things we want (which is quite a few!). I've written down a summary of the use cases and an explanation of the ideas behind the API here: Amongst others, I introduce the concept of a "recipe" and worked out a POC for this. I have no idea if this idea resonates with you guys, feedback is very welcome! I find it hard to come up with naming for the different concepts (factories, recipes, functions). See also the folder !!! I really need your inputs !!! |
@josdejong I read over your readme, and I like it! It looks like a good way to design things. "recipe" is kinda an awkward term, but, like you said, naming things is hard. I don't have a better suggestion. One thing I found confusing was the following snippet: const { divide, sin, pi } = create({ addRecipe, divideRecipe }) Where are |
Thanks for your feedback @flaviut!
Ah sorry, I had not correctly updated all code examples in the test-tree-shaking readme I see, fixed now.
Yeah, I'm in search of better naming instead of |
Some more naming ideas instead of import { create, divideDependencies, sinDependencies, piDependencies } from 'mathjs'
const { divide, sin, pi } = create({
divideDependencies,
sinDependencies,
piDependencies
}) |
All dependency collections are now worked out (automatically generated! :) ). All examples are working again. Still lots of details to work out, but we're close to releasing a first beta to start collecting feedback. |
hm. Karma tests are failing in See:
|
~~ So webpack is trying to bundle our node only tests and failing when it reaches Wrong. |
See my review in #1319 |
After a half year of hard work I've just published
How can you help?
How to try the new stuff?
Still to do before v6.0.0 can be released:
Ping @kevjin |
Argh! I just noticed that the beta release is missing the new index files (must me on a whitelist). Fill fix that asap tomorrow with a new beta release. |
ES6 indexes working now in |
I've just merged |
Awesome job @josdejong :) |
I've now worked out the scripts to automatically generate all entry/index files needed for ES6 import and tree shaking. The files So, to create a new function now, all you have to do is create a new file containing the function implementation, and add an export of it in |
I've just published |
I've published |
I've published Tomorrow I want to publish a blog post about it. |
Blog post available here: https://josdejong.com/blog/2019/06/08/mathjs-v6-a-monolith-turned-modular/ will close this issue now :) |
This would
The text was updated successfully, but these errors were encountered: