`PolynomialPathFitter` and other utilities for `FunctionBasedPath` #3581

nickbianco · 2023-10-20T00:07:11Z

Fixes issue #3390

Brief summary of changes

This PR introduces PolynomialPathFitter, a new utility class for creating FunctionBasedPaths by fitting the coefficients of MultivariatePolynomialFunctions to path length and moment arm samples taken from a model with geometry-based paths. Given a model, a table of coordinate values, and a set of (optional) user settings, this class automatically samples path lengths and moment arms from the model based on Latin hypercube sampling and determins the polynomial coefficients using a least-squares fit. In addition to the new class, various other smaller changes are included to make everything work:

Added ModelFactor::replacePathsWithFunctionBasedPaths and ModOpReplacePathsWithFunctionBasedPaths.
Added calcTermValues and calcTermDerivatives to MultivariatePolynomialFunction.
Added Object::getProperty(const std::string& name) and Object::updProperty(const std::string& name).
Added TypeHelpers to Property for SimTK::Vec2 types.
Added SimulationUtilities::appendCoupledCoordinateValues and TabOpAppendCoupledCoordinateValues.
Added SimulationUtilities::appendCoordinateValueDerivatives and TabOpAppendCoordinateValueDerivatives.
Minor updates to FunctionBasedPath.
Other various minor drive-by fixes that I encountered along the way.

Testing I've completed

Added testPolynomialPathFitter.cpp.

Looking for feedback on...

Anything and everything.

CHANGELOG.md (choose one)

updated.

This change is

…utation code

aymanhab

Reviewed 5 of 29 files at r1, all commit messages.
Reviewable status: 5 of 29 files reviewed, 1 unresolved discussion (waiting on @adamkewley and @nickbianco)

a discussion (no related file):
I think this will be extremely useful to make the adoption easier 👍 . Some questions that may have been addressed or documented elsewhere that I found myself asking with every call so would be good to clear up or if you can point to existing documentation if exists already:

Do you have to do the replacement for all paths or you can be selective and do mix of GeometryBased and FunctionBased?
Are there checks that the functions passed-in correspond and map to components that are actually GeometryBased paths, nothing extra, and nothing missing (depending on previous item on this list).
Assuming both paths exist in the model at some point, using _path as a suffix seems more confusing when we call both classes "*Path", I'd use something that includes "function" or "fit" unless I'm not clear on what's being named/renamed.
Will proceed with the review once I understand the context based on the above. Thanks

nickbianco

Reviewable status: 5 of 29 files reviewed, 1 unresolved discussion (waiting on @adamkewley and @aymanhab)

a discussion (no related file):

Previously, aymanhab (Ayman Habib) wrote…

I think this will be extremely useful to make the adoption easier 👍 . Some questions that may have been addressed or documented elsewhere that I found myself asking with every call so would be good to clear up or if you can point to existing documentation if exists already:

Do you have to do the replacement for all paths or you can be selective and do mix of GeometryBased and FunctionBased?

Are there checks that the functions passed-in correspond and map to components that are actually GeometryBased paths, nothing extra, and nothing missing (depending on previous item on this list).

Assuming both paths exist in the model at some point, using _path as a suffix seems more confusing when we call both classes "*Path", I'd use something that includes "function" or "fit" unless I'm not clear on what's being named/renamed.
Will proceed with the review once I understand the context based on the above. Thanks

You can be selective. We save the fitted polynomial paths to an XML file, so you can modify the XML file or load specific paths in to the model with the API. We could also add an exclude_paths option on ModOpReplacePathWithFunctionBasedPaths.
This class will only fit paths that derive from AbstractPath and (currently) will throw an exception if the model passed in already has FunctionBasedPaths. We could be more granular and only fit paths that are GeometryPath (or other future geometry-based paths), but for now I just assume that users do not intend to pass in a model with a FunctionBasedPath.
By both paths, do you mean supporting both a GeometryPath and a FunctionBasedPath for the same force object? Then yes that name convention might be confusing. But that could be handled in a future PR I think.

aymanhab

Reviewed 21 of 29 files at r1.
Reviewable status: 26 of 29 files reviewed, 10 unresolved discussions (waiting on @adamkewley and @nickbianco)

a discussion (no related file):

Previously, nickbianco (Nicholas Bianco) wrote…

You can be selective. We save the fitted polynomial paths to an XML file, so you can modify the XML file or load specific paths in to the model with the API. We could also add an exclude_paths option on ModOpReplacePathWithFunctionBasedPaths.

This class will only fit paths that derive from AbstractPath and (currently) will throw an exception if the model passed in already has FunctionBasedPaths. We could be more granular and only fit paths that are GeometryPath (or other future geometry-based paths), but for now I just assume that users do not intend to pass in a model with a FunctionBasedPath.

By both paths, do you mean supporting both a GeometryPath and a FunctionBasedPath for the same force object? Then yes that name convention might be confusing. But that could be handled in a future PR I think.

Thanks for the clarification @nickbianco now I can proceed.

OpenSim/Actuators/ModelFactory.cpp line 318 at r1 (raw file):

            
        // Get the force component associated with this path.
        OPENSIM_THROW_IF(!model.hasComponent<Force>(path.getName()),

Have you considered using Path or AbstractPath instead of Force? If so we use more specific class in updComponent<> here so we don't end up catching unrelated components? I think there're methods to go from Path to the owner component safely and you're only using Object level methods anyway.

OpenSim/Actuators/PolynomialPathFitter.h line 46 at r1 (raw file):

    OpenSim_DECLARE_PROPERTY(coordinate_path, std::string,
            "The path to the bounded coordinate in the model.")
    OpenSim_DECLARE_PROPERTY(bounds, SimTK::Vec2,

coordinate may not be "bounded" in the model, would it be clearer to say path to the coordinate to be bounded during fitting?

OpenSim/Actuators/PolynomialPathFitter.h line 80 at r1 (raw file):

 * `setMomentArmsThreshold` method determines whether or not a path depends on a
 * model coordinate. In other words, the moment arm of a path with respect to a
 * particular coordinate must be greater than this value to be included during

using absolute value?

OpenSim/Actuators/PolynomialPathFitter.h line 97 at r1 (raw file):

 * the Latin hypercube sampling algorithm used to sample coordinate values for
 * path fitting.
 *

With all these tolerances I'm assuming the model is assumed to be in meters so a model of a small bird or a dinosaur may not work out of the box? Is this something to account for or include a disclaimer?

OpenSim/Actuators/PolynomialPathFitter.h line 400 at r1 (raw file):

    void setParallel(int numThreads);
    /// @copydoc setParallel()
    int getParallel() const;

The name setParallel is a bit unexpected naming convention, I'd expect either a bool or numParallelThreads(number) but if this convention is used elsewhere in Moco then would be fine to keep

OpenSim/Common/MultivariatePolynomialFunction.h line 103 at r1 (raw file):

    /// Get a vector of the terms in the polynomial function.
    SimTK::Vector getTermValues(const SimTK::Vector& x) const;
    /// Get a vector of the derivatives of the terms in the polynomial function.

Do you think returning a fresh copy could be a performance bottleneck? Is this exercised during simulation?

OpenSim/Common/Object.h line 347 at r1 (raw file):

    template <class T> const Property<T>&
    getProperty(const std::string& name) const;

For consistency with naming of other methods in the file I'd call this method getPropertyByName. Same for other new methods that do name lookup. Curious why this was needed now

OpenSim/Common/Object.h line 355 at r1 (raw file):

    /** @copydoc updProperty(const PropertyIndex&) **/
    template <class T> Property<T>&
    updProperty(const std::string& name);

This looks fine but I'm wondering if this is part of the drive-by changes or are relevant to this PR functionality. Thank you @nickbianco

OpenSim/Simulation/SimulationUtilities.h line 401 at r1 (raw file):

        TimeSeriesTable& table, const Model& model,
        bool overwriteExistingColumns = true);

Should the method name include "speeds" vs "derivatives"? Trying to consolidate the comment with the naming

adamkewley · 2023-10-25T18:37:49Z

Apologies for the delay on looking at this, but I've been a bit busy, and now I'm away

I'd nominate @pepbos to have a quick look, given that he's interested in similar ideas etc. and knows his way around C++ 😉

nickbianco

Reviewable status: 20 of 29 files reviewed, 10 unresolved discussions (waiting on @aymanhab and @pepbos)

a discussion (no related file):

Previously, aymanhab (Ayman Habib) wrote…

Thanks for the clarification @nickbianco now I can proceed.

Thanks @aymanhab, ready for review again.

OpenSim/Actuators/ModelFactory.cpp line 318 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Have you considered using Path or AbstractPath instead of Force? If so we use more specific class in updComponent<> here so we don't end up catching unrelated components? I think there're methods to go from Path to the owner component safely and you're only using Object level methods anyway.

The problem is that AbstractPaths usually do not have unique names/paths in the model. So I could search through all AbstractPaths in the model, but I'd end up having to check the Force name anyway. The subsequent line checks if the path property exists in the component.

OpenSim/Actuators/PolynomialPathFitter.h line 46 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

coordinate may not be "bounded" in the model, would it be clearer to say path to the coordinate to be bounded during fitting?

Done.

OpenSim/Actuators/PolynomialPathFitter.h line 80 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

using absolute value?

Done.

OpenSim/Actuators/PolynomialPathFitter.h line 97 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

With all these tolerances I'm assuming the model is assumed to be in meters so a model of a small bird or a dinosaur may not work out of the box? Is this something to account for or include a disclaimer?

Done.

OpenSim/Actuators/PolynomialPathFitter.h line 400 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

The name setParallel is a bit unexpected naming convention, I'd expect either a bool or numParallelThreads(number) but if this convention is used elsewhere in Moco then would be fine to keep

Done.

OpenSim/Common/MultivariatePolynomialFunction.h line 103 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Do you think returning a fresh copy could be a performance bottleneck? Is this exercised during simulation?

I'm returning a temporary SimTK::Vector constructed inside this method, so returning it like this should be optimized by the compiler.

https://stackoverflow.com/questions/4986673/is-it-necessary-to-stdmove-in-a-return-statement-and-should-you-return-rvalue

OpenSim/Common/Object.h line 347 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

For consistency with naming of other methods in the file I'd call this method getPropertyByName. Same for other new methods that do name lookup. Curious why this was needed now

Done.

OpenSim/Common/Object.h line 355 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

This looks fine but I'm wondering if this is part of the drive-by changes or are relevant to this PR functionality. Thank you @nickbianco

They are used in PolynomialPathFitter and ModelFactory::replacePathsWithFunctionBasedPaths.

OpenSim/Simulation/SimulationUtilities.h line 401 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Should the method name include "speeds" vs "derivatives"? Trying to consolidate the comment with the naming

Added the "AsSpeeds" suffix for clarity.

aymanhab

Looks great @nickbianco Just a couple clarifications and then it's ready to go 👍

Reviewed 9 of 9 files at r2, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @nickbianco and @pepbos)

OpenSim/Actuators/PolynomialPathFitter.h line 157 at r2 (raw file):

 *
 * It is highly recommended to use the files printed to the output directory to
 * evaluate the quality of the fitted paths. Depending on the quality of the

Maybe documented else where but a pointer to what files are printed and how to use would be helpful.

OpenSim/Actuators/PolynomialPathFitter.h line 190 at r2 (raw file):

     * the model to contain at least one path object derived from `AbstractPath`
     * and does not already contain any `FunctionBasedPath` objects. The bounds
     * for clamped coordinates are obeyed during the fitting process. Locked

To clarify, is the implication that users can't convert to FunctionBasedPaths one muscle/path at a time? Definitely this is cleaner/easier for book-keeping and they can iteratively add and refit, just confirming supported workflow.

OpenSim/Actuators/PolynomialPathFitter.h line 309 at r2 (raw file):

    /**
     * The global bounds (in degrees) that determine the minimum and maximum

Anything to say here about translational coordinates?

OpenSim/Actuators/PolynomialPathFitter.cpp line 2 at r2 (raw file):

/* -------------------------------------------------------------------------- *
 *                    OpenSim:  PolynomialPathFitter.h                        *

.cpp :)

OpenSim/Actuators/Test/testPolynomialPathFitter.cpp line 131 at r2 (raw file):

        REQUIRE_THROWS_WITH(fitter.run(), Catch::Contains(
            "Expected 'maximum_polynomial_order' to be at most 9"));
    }

Very nicely done/tested @nickbianco

nickbianco

@aymanhab ready for review.

Reviewable status: 27 of 29 files reviewed, 8 unresolved discussions (waiting on @aymanhab and @pepbos)

OpenSim/Actuators/PolynomialPathFitter.h line 157 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Maybe documented else where but a pointer to what files are printed and how to use would be helpful.

Done.

OpenSim/Actuators/PolynomialPathFitter.h line 190 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

To clarify, is the implication that users can't convert to FunctionBasedPaths one muscle/path at a time? Definitely this is cleaner/easier for book-keeping and they can iteratively add and refit, just confirming supported workflow.

Yes, that correct. However, every path is fitted independently, so it would be easy enough for users to remove muscles from the model that they do not want in the fitting process.

OpenSim/Actuators/PolynomialPathFitter.h line 309 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Anything to say here about translational coordinates?

Done.

OpenSim/Actuators/PolynomialPathFitter.cpp line 2 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

.cpp :)

Done.

OpenSim/Actuators/Test/testPolynomialPathFitter.cpp line 131 at r2 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Very nicely done/tested @nickbianco

Done.

pepbos

Interesting stuff!
The different uses of "path" become a bit confusing, especially if things are done at runtime, rather than checked at compile time.

pepbos · 2023-11-01T06:37:38Z

OpenSim/Actuators/ModelFactory.cpp

@@ -309,3 +309,26 @@ void ModelFactory::createReserveActuators(Model& model, double optimalForce,
    }
 }

+void ModelFactory::replacePathsWithFunctionBasedPaths(Model& model,
+            const Set<FunctionBasedPath>& functionBasedPaths) {


The different meanings of "path" becomes confusing in this function...

pepbos · 2023-11-01T06:46:45Z

OpenSim/Actuators/ModelFactory.cpp

+        OPENSIM_THROW_IF(!model.hasComponent<Force>(path.getName()), 
+                Exception, "Model does not contain a Force at path {}.", 
+                path.getName());
+        auto& force = model.updComponent<Force>(path.getName());


Seems odd that the name of the FunctionBasedPath is equal to the path of a component in a model. Is there a reason for this?

pepbos · 2023-11-01T06:48:38Z

OpenSim/Actuators/ModelFactory.cpp

+        // Get the force component associated with this path.
+        OPENSIM_THROW_IF(!model.hasComponent<Force>(path.getName()), 
+                Exception, "Model does not contain a Force at path {}.", 
+                path.getName());


This Throw is already performed at updComponent calls, with similar message. Perhaps add info if needed, or remove the throw here?

pepbos · 2023-11-01T06:52:28Z

OpenSim/Actuators/ModelFactory.cpp

+        // Check that the force has a path property.
+        OPENSIM_THROW_IF(
+                !force.hasProperty("path"), Exception,
+                "Force {} does not have a path property.", path.getName());


This throw does add new info, but does not use the throw from updProperty. Perhaps use try?

pepbos · 2023-11-01T07:01:31Z

OpenSim/Actuators/ModelFactory.cpp

+                "Force {} does not have a path property.", path.getName());
+
+        // Update the path.
+        path.setName(fmt::format("{}_path", force.getName()));


The "path" to a PathActuator as the name of a FunctionBasedPath's name, postfixed with _path seems confusing.
If there are more functions out there that work like this it might become more confusing.
Is this postfix with _path a fix or feature? Why not use the name the user gave to this FunctionBasedPath?

pepbos · 2023-11-01T07:03:46Z

OpenSim/Actuators/ModelFactory.cpp

+
+        // Update the path.
+        path.setName(fmt::format("{}_path", force.getName()));
+        force.updPropertyByName<AbstractPath>("path").setValue(path);


Use try here?

pepbos · 2023-11-01T07:28:52Z

OpenSim/Actuators/ModelFactory.cpp

+
+        // Update the path.
+        path.setName(fmt::format("{}_path", force.getName()));
+        force.updPropertyByName<AbstractPath>("path").setValue(path);


Does this name never change? Since "path" seems to be pretty ambiguous, someone might change the name of the property "path".

pepbos

A more general question: In MultivariatePolynomialFunction, why are you inheriting from the templated Function, and not Function_<double>?

pepbos · 2023-11-01T07:54:36Z

OpenSim/Common/MultivariatePolynomialFunction.cpp

        OPENSIM_THROW_IF(coefficients.size() != coeff_nr, Exception,
                "Expected {} coefficients but got {}.", coeff_nr,
                coefficients.size());
    }
-    T calcValue(const SimTK::Vector& x) const override {
+
+    SimTK::Vector_<T> calcTermValues(const SimTK::Vector& x) const {


Avoid heap allocation.

pepbos · 2023-11-01T08:03:56Z

OpenSim/Common/MultivariatePolynomialFunction.cpp

        std::array<int, 6> nq{{0, 0, 0, 0, 0, 0}};
-        T value = static_cast<T>(0);
+        SimTK::Vector_<T> derivatives = static_cast<SimTK::Vector_<T>>(


Are you sure this doesnt simply cast the Vector, not the data?

pepbos · 2023-11-01T08:06:11Z

OpenSim/Common/MultivariatePolynomialFunction.cpp

@@ -163,57 +169,57 @@ class SimTKMultivariatePolynomial : public SimTK::Function_<T> {
                                if (derivComponent[0] == 0) {
                                    nqNonNegative = nq[0] - 1;
                                    if (nqNonNegative < 0) nqNonNegative = 0;
-                                    T valueP = nq[0] * std::pow(x[0], nqNonNegative);
+                                    T derivP = nq[0] * std::pow(x[0], nqNonNegative);


The code above looks a lot like the code at the constructor: Perhaps worth reusing it...

pepbos · 2023-11-01T08:07:38Z

OpenSim/Common/MultivariatePolynomialFunction.cpp

+
+SimTK::Vector MultivariatePolynomialFunction::getTermValues(
+        const SimTK::Vector& x) const {
+    if (_function == nullptr)


if (!_function) {

pepbos · 2023-11-01T08:09:15Z

OpenSim/Common/MultivariatePolynomialFunction.cpp

                                    }
-                                    value += valueP * coefficients[coeff_nr];
+                                    derivatives[coeff_nr] = derivP;


A few comments would really help reading this large piece of code.

nickbianco

Reviewable status: 25 of 30 files reviewed, 20 unresolved discussions (waiting on @aymanhab and @pepbos)

OpenSim/Common/MultivariatePolynomialFunction.cpp line 79 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

Avoid heap allocation.

values is initialized the function so it should be allocated on the stack. Unless static casting (which is now removed, see below) does funny things with stack vs heap allocation that I'm not aware of.

OpenSim/Common/MultivariatePolynomialFunction.cpp line 134 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

Are you sure this doesnt simply cast the Vector, not the data?

Updated to avoid the static_cast.

OpenSim/Common/MultivariatePolynomialFunction.cpp line 172 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

The code above looks a lot like the code at the constructor: Perhaps worth reusing it...

Done.

OpenSim/Common/MultivariatePolynomialFunction.cpp line 222 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

A few comments would really help reading this large piece of code.

Done.

OpenSim/Common/MultivariatePolynomialFunction.cpp line 268 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

if (!_function) {

Done.

nickbianco

Thanks @pepbos! Ready for review again. I tried to make replacePathsWithFunctionBasedPaths less confusing, but happy to make additional changes if necessary.

In MultivariatePolynomialFunction, why are you inheriting from the templated Function, and not Function_<double>?

I didn't originally write this code, but it looks like inheriting from Function_<T> is the intended interface in Simbody.

Reviewable status: 23 of 30 files reviewed, 20 unresolved discussions (waiting on @aymanhab and @pepbos)

OpenSim/Actuators/ModelFactory.cpp line 313 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

The different meanings of "path" becomes confusing in this function...

I tried to clarify in the doc comments.

OpenSim/Actuators/ModelFactory.cpp line 320 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

This Throw is already performed at updComponent calls, with similar message. Perhaps add info if needed, or remove the throw here?

Done.

OpenSim/Actuators/ModelFactory.cpp line 321 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

Seems odd that the name of the FunctionBasedPath is equal to the path of a component in a model. Is there a reason for this?

See my comment below.

OpenSim/Actuators/ModelFactory.cpp line 326 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

This throw does add new info, but does not use the throw from updProperty. Perhaps use try?

Done.

OpenSim/Actuators/ModelFactory.cpp line 329 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

The "path" to a PathActuator as the name of a FunctionBasedPath's name, postfixed with _path seems confusing.
If there are more functions out there that work like this it might become more confusing.
Is this postfix with _path a fix or feature? Why not use the name the user gave to this FunctionBasedPath?

Naming the FunctionBasedPath is actually unnecessary since all path-based forces will store it using a property named 'path'. When the force is serialized it will then have the name 'path'. The names for each FunctionBasedPath in the XML file is so we know which Force to assign the FunctionBasedPath to. It might be more proper to have a small helper class that stores the FunctionBasedPath and the name of the Force it is assigned to, but this implementation seemed reasonable to me.

OpenSim/Actuators/ModelFactory.cpp line 330 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

Use try here?

Done.

OpenSim/Actuators/ModelFactory.cpp line 330 at r3 (raw file):

Previously, pepbos (pepbos) wrote…

Does this name never change? Since "path" seems to be pretty ambiguous, someone might change the name of the property "path".

As I said in the other comment, all path-based forces (i.e., PathSpring, PathActuator, Ligament, and Blankevoort1991Ligament) store paths using a property named 'path'. I guess someone could create a new path-based force object that uses a different property name, which would cause a failure here. As an alternative, since the force should only have one path, I could somehow look up the PropertyIndex for the path and update it that way.

aymanhab

Reviewed 2 of 2 files at r3, 1 of 3 files at r4, 1 of 3 files at r5, all commit messages.
Reviewable status: 26 of 30 files reviewed, 12 unresolved discussions (waiting on @pepbos)

nickbianco · 2023-11-08T16:58:09Z

Merging with @aymanhab's approval and passing tests. I have a few other fixes, but they will come in a follow up PR.

nickbianco added 30 commits September 18, 2023 15:00

Draft utilities for adding FunctionBasedPaths

4514fba

Merge branch 'function_based_path' into function_based_path_utils

fd5ec86

Merge branch 'function_based_path' into function_based_path_utils

e700333

Fleshing out path utilities

4b48e74

Merge branch 'function_based_path' into function_based_path_utils

21b44d1

Merge branch 'function_based_path' into function_based_path_utils

2ad259f

Simplify the new ModelOperator // start fleshing out path length comp…

8a714ab

…utation code

Merge branch 'function_based_path' into function_based_path_utils

60ffecc

finalize path length and moment arm computation utility

307a321

Merge branch 'function_based_path' into function_based_path_utils

78335de

First pass at polynomial fitting utility

f23a75c

Merge branch 'function_based_path' into function_based_path_utils

9b44256

Testing the fitting utility

5c7d505

Merge branch 'function_based_path' into function_based_path_utils

2991448

Merge branch 'function_based_path' into function_based_path_utils

d4376b3

Fitting utility is working!

57ebe85

Fix merge conflict

9450572

Merge branch 'function_based_path' into function_based_path_utils

c4f329e

Prototyping a class for generating Latin hypercube designs

a336259

Re-arranging the new utilities

d90672e

Merge branch 'latin_hypercube_design' into function_based_path_utils

9916560

Saving current progress on LHS designs

84f16ae

Merge branch 'latin_hypercube_design' into function_based_path_utils

ac3aaca

Remove outdated Latin hypercube code

8a5ed38

Merge branch 'latin_hypercube_design' into function_based_path_utils

7fe2852

Start fleshing out a FunctionBasedPathFitter class

d16a2ba

Move fitter to Actuators and rename to 'PolynomialPathFitter'

579f18f

Removing unnecessary whitespace changes

3660f8c

Undo more whitespace changes

149b18e

Last set of whitespace undos

f01cbd7

nickbianco requested a review from aymanhab October 20, 2023 22:07

nickbianco changed the title ~~[WIP] PolynomialPathFitter and other utilities for FunctionBasedPath~~ PolynomialPathFitter and other utilities for FunctionBasedPath Oct 20, 2023

Set parallelization in test for CI

2e4b7e5

aymanhab requested changes Oct 24, 2023

View reviewed changes

nickbianco commented Oct 24, 2023

View reviewed changes

aymanhab requested changes Oct 25, 2023

View reviewed changes

nickbianco requested review from pepbos and removed request for adamkewley October 25, 2023 20:04

Addressing first round of review comments

fd01001

nickbianco commented Oct 25, 2023

View reviewed changes

Fix testPolynomialPathFitter

2d1a6fe

aymanhab requested changes Oct 26, 2023

View reviewed changes

Addressing review comments round 2

27a3c4d

nickbianco commented Oct 26, 2023

View reviewed changes

Merge branch 'main' into function_based_path_utils

5ed1300

pepbos reviewed Nov 1, 2023

View reviewed changes

Updates to MultivariatePolynomialFunction

21454e1

nickbianco commented Nov 2, 2023

View reviewed changes

Make 'replacePathsWithFunctionBasedPaths' less confusing

d9fd62f

nickbianco commented Nov 2, 2023

View reviewed changes

Merge branch 'main' into function_based_path_utils

4b9c643

nickbianco requested review from pepbos and aymanhab November 3, 2023 23:55

aymanhab approved these changes Nov 7, 2023

View reviewed changes

Merge branch 'main' into function_based_path_utils

1f55352

nickbianco merged commit eec8be7 into main Nov 8, 2023
5 of 7 checks passed

nickbianco deleted the function_based_path_utils branch November 8, 2023 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`PolynomialPathFitter` and other utilities for `FunctionBasedPath` #3581

`PolynomialPathFitter` and other utilities for `FunctionBasedPath` #3581

nickbianco commented Oct 20, 2023 •

edited

Loading

aymanhab left a comment

nickbianco left a comment

aymanhab left a comment

adamkewley commented Oct 25, 2023

nickbianco left a comment

aymanhab left a comment

nickbianco left a comment

pepbos left a comment

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos left a comment

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

pepbos Nov 1, 2023

nickbianco left a comment

nickbianco left a comment

aymanhab left a comment

nickbianco commented Nov 8, 2023

PolynomialPathFitter and other utilities for FunctionBasedPath #3581

PolynomialPathFitter and other utilities for FunctionBasedPath #3581

Conversation

nickbianco commented Oct 20, 2023 • edited Loading

Brief summary of changes

Testing I've completed

Looking for feedback on...

CHANGELOG.md (choose one)

aymanhab left a comment

Choose a reason for hiding this comment

nickbianco left a comment

Choose a reason for hiding this comment

aymanhab left a comment

Choose a reason for hiding this comment

adamkewley commented Oct 25, 2023

nickbianco left a comment

Choose a reason for hiding this comment

aymanhab left a comment

Choose a reason for hiding this comment

nickbianco left a comment

Choose a reason for hiding this comment

pepbos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pepbos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickbianco left a comment

Choose a reason for hiding this comment

nickbianco left a comment

Choose a reason for hiding this comment

aymanhab left a comment

Choose a reason for hiding this comment

nickbianco commented Nov 8, 2023

`PolynomialPathFitter` and other utilities for `FunctionBasedPath` #3581

`PolynomialPathFitter` and other utilities for `FunctionBasedPath` #3581

nickbianco commented Oct 20, 2023 •

edited

Loading